diff --git a/hack/capture-dataplane.bats b/hack/capture-dataplane.bats index f9fd829c7..631cb9ff0 100644 --- a/hack/capture-dataplane.bats +++ b/hack/capture-dataplane.bats @@ -15,6 +15,8 @@ # succeeded, unknown when none ran or none could. # - pod_filter_affected -- retain scheduled NotReady pods even without IPs, # and only from whole rows. +# - pod_first_ready -- pick the healthy-pod baseline for one node out +# of the cluster-wide snapshot, whole rows only. # # The tests come in two shapes, and neither needs a cluster. # @@ -57,25 +59,150 @@ E2E_CAPTURE_DATAPLANE_LIB=1 @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')" + 'tenant|no-ip||node-a|False|Pending||eol' \ + 'tenant|with-ip|192.0.2.80|node-b|False|Running||eol')" 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 }' + printf '%s\n' "$out" | awk '$0 == "tenant|no-ip||node-a|False|Pending||eol" { 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')" + 'tenant|ready|192.0.2.81|node-a|True|Running||eol' \ + 'tenant|unscheduled|||False|Pending||eol' \ + 'tenant|succeeded|192.0.2.82|node-b|False|Succeeded||eol' \ + 'tenant|failed|192.0.2.83|node-b|False|Failed||eol')" [ -z "$(printf '%s\n' "$rows" | pod_filter_affected)" ] } +@test "pod_first_ready picks the first Ready Running pod on the named node" { + # Rows are the cluster-wide snapshot, so the filter scopes to a node itself: + # ns|name|podIP|nodeName|ready|phase|hostNetwork. + rows="$(printf '%s\n' \ + 'tenant|not-ready|192.0.2.90|node-a|False|Running||eol' \ + 'tenant|pending|192.0.2.91|node-a|True|Pending||eol' \ + 'tenant|other-node|192.0.2.94|node-b|True|Running||eol' \ + 'tenant|healthy-1|192.0.2.92|node-a|True|Running||eol' \ + 'tenant|healthy-2|192.0.2.93|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|healthy-1|192.0.2.92" ] +} + +@test "pod_first_ready ignores an eligible pod on a different node" { + # The scoping is the whole reason this reads the cluster-wide list safely: a + # baseline from another machine would compare the wedged pod's route against + # a route that never shared a kernel with it. + rows="$(printf '%s\n' 'tenant|healthy|192.0.2.95|node-b|True|Running||eol')" + + [ -z "$(printf '%s\n' "$rows" | pod_first_ready node-a)" ] +} + +@test "pod_first_ready emits nothing when no pod on the node is Ready and Running" { + rows="$(printf '%s\n' \ + 'tenant|not-ready|192.0.2.94|node-a|False|Running||eol' \ + 'tenant|pending|192.0.2.95|node-a|True|Pending||eol')" + + [ -z "$(printf '%s\n' "$rows" | pod_first_ready node-a)" ] +} + +@test "pod_first_ready emits nothing on empty input" { + [ -z "$(printf '' | pod_first_ready node-a)" ] +} + +@test "pod_first_ready rejects a hostNetwork row cut at the separator" { + # The half a value check cannot reach on its own. A stream cut RIGHT AFTER the + # sixth separator leaves the hostNetwork column empty, and empty is + # byte-identical to the omitempty-false the filter must accept -- so `$7 == + # ""` alone seats a hostNetwork pod whose `true` was cut away entirely. The + # trailing constant is what separates them, and the two halves of that guard + # catch different cuts, so both are exercised here: + # row 1 loses its tail before the last separator -> NF is 7, not 8 + # row 2 keeps the count but the sentinel is cut -> NF is 8, $8 is not eol + # Neither may be picked; the whole row behind them must be. + rows="$(printf '%s\n' \ + 'cozy-cilium|cilium-abcde|192.0.2.10|node-a|True|Running|' \ + 'cozy-cilium|cilium-fghij|192.0.2.13|node-a|True|Running||eo' \ + 'tenant|workload|192.0.2.12|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|workload|192.0.2.12" ] +} + +@test "pod_first_ready rejects a hostNetwork row cut inside its last value" { + # The one filter here where a truncated last value must NOT be kept. + # `tru` carries all eight fields and passes every other gate, so a deny-list + # (`$7 != "true"`) accepts it -- and because this filter stops at the row it + # takes, the baseline then IS a hostNetwork pod, whose podIP is the node's own + # address. That is the fingerprint the exclusion exists to keep out of the + # comparison, so the control would be the thing it excludes. The allow-list + # form rejects any value that is not an exact known state. + rows="$(printf '%s\n' \ + 'cozy-cilium|cilium-abcde|192.0.2.10|node-a|True|Running|tru|eol' \ + 'tenant|workload|192.0.2.11|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|workload|192.0.2.11" ] +} + +@test "pod_first_ready skips a Ready pod that has no podIP yet" { + # The filter stops at the row it takes, so an addressless pod does not just + # produce a thin baseline -- it consumes the only pick and the pod behind it + # is never considered. + rows="$(printf '%s\n' \ + 'tenant|no-ip-yet||node-a|True|Running||eol' \ + 'tenant|healthy|192.0.2.99|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|healthy|192.0.2.99" ] +} + +@test "pod_first_ready drops a pod row cut off mid-record" { + # The cut has to land where the fragment still satisfies every value test, or + # the row is rejected for an unrelated reason and the count is never + # consulted: here the stream ends right after `Running`, one separator short + # of the hostNetwork column. + rows="$(printf '%s\n' \ + 'tenant|cut-off|192.0.2.9|node-a|True|Running' \ + 'tenant|healthy|192.0.2.98|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|healthy|192.0.2.98" ] +} + +@test "pod_first_ready skips a hostNetwork pod even when it is Ready Running and first" { + # cilium-agent / kube-ovn-cni / ovs-ovn run hostNetwork on every node, and a + # hostNetwork pod's podIP IS the node's own address -- picking one as the + # baseline would resolve to a local route for an entirely ordinary reason, + # which is the same fingerprint the address-attribution capture exists to + # recognise, so the baseline would defeat itself. + rows="$(printf '%s\n' \ + 'cozy-cilium|cilium-agent-abcde|192.0.2.10|node-a|True|Running|true|eol' \ + 'tenant|workload|192.0.2.96|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|workload|192.0.2.96" ] +} + +@test "pod_first_ready treats a blank hostNetwork column as NOT hostNetwork" { + # The API omits spec.hostNetwork (omitempty) when it is false/unset, so + # kubectl's jsonpath emits an empty string, not the literal "false". + rows="$(printf '%s\n' 'tenant|workload|192.0.2.97|node-a|True|Running||eol')" + + out="$(printf '%s\n' "$rows" | pod_first_ready node-a)" + + [ "$out" = "tenant|workload|192.0.2.97" ] +} + @test "runtime checks LoadBalancers when there are no affected pods" { tmp=$(mktemp -d) trap 'rm -rf "$tmp"' EXIT @@ -138,11 +265,13 @@ EOF } @test "pod_filter_affected drops a pod row cut off mid-record" { - # The same cut over the six-field pod jsonpath. The fragment ends inside the + # The same cut over the eight-field pod jsonpath. The fragment ends inside the # nodeName column, so the half-parsed `nod` reads as a scheduled pod on a node - # of that name, and the capture opens a node-nod.txt for it. + # of that name, and the capture opens a node-nod.txt for it. Note the cut has + # to land in a column this filter reads: hostNetwork is last now, and a cut + # inside THAT one leaves every column this filter decides on intact. rows="$(printf '%s\n' \ - 'tenant|wedged|192.0.2.80|node-b|False|Running' \ + 'tenant|wedged|192.0.2.80|node-b|False|Running||eol' \ 'tenant|cut||nod')" out="$(printf '%s\n' "$rows" | pod_filter_affected)" @@ -381,7 +510,7 @@ dp_hanging_kubectl_dir() { #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; esac done sleep 300 @@ -466,7 +595,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; esac done # The behaviour under test: client-go's evalArray has no allowMissingKeys @@ -508,7 +637,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; esac done case "$*" in @@ -556,7 +685,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; esac done case "$*" in @@ -683,7 +812,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; esac done case "$*" in @@ -717,7 +846,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|healthy|10.0.0.9|node-a|True|Running'; exit 1 ;; + pods) echo 'tenant-test|healthy|10.0.0.9|node-a|True|Running||eol'; exit 1 ;; esac done exit 0 @@ -744,7 +873,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; esac done case "$*" in @@ -765,6 +894,413 @@ STUB rm -rf "$d" } +@test "the host netns capture names every address, not only ovn0" { + # The reason this commit exists: when a podIP turns out to be a local address + # of the host netns, `ip addr show ovn0` cannot say which interface holds it. + # Nothing else in this suite reaches these two captures -- deleting both left + # every other test green -- so without this the collector could quietly lose + # the evidence the change was written to collect. + # + # Asserted on the kubectl argv, and the section headers are the secondary + # check rather than the primary one. A header is an `echo` the script emits + # before the exec, so dropping the exec and keeping the label leaves an empty + # section behind and every header grep green -- either capture alone, or both + # together, and the whole suite stays green. The argv also pins WHERE the + # capture runs -- `-c cni-server`, the container that shares the host netns -- + # which is half the property and one a header cannot carry at all. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +case "$*" in + *'ip -o addr show'*|*'ip route show table local'*) echo "$*" >>"$STUB_CALLS" ;; +esac +for a in "$@"; do + case $a in + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; + esac +done +case "$*" in + *'app=kube-ovn-cni'*) echo 'cni-abc'; exit 0 ;; +esac +exit 0 +STUB + chmod +x "$d/bin/kubectl" + STUB_CALLS="$d/calls" PATH="$d/bin:$PATH" timeout 120 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + [ -f "$d/out/node-node-a.txt" ] + # Positive control: the host-netns section ran, so a missing address capture + # below is that capture's absence and not the whole block being skipped. + grep -q 'host netns: ip rule' "$d/out/node-node-a.txt" + # The commands the collector actually issued into the host netns ... + grep -q -- '-c cni-server -- ip -o addr show' "$d/calls" + grep -q -- '-c cni-server -- ip route show table local' "$d/calls" + # ... and the labels a reader of the artifact finds them under. + grep -q 'ip -o addr show, all interfaces' "$d/out/node-node-a.txt" + grep -q 'ip route show table local' "$d/out/node-node-a.txt" + rm -rf "$d" +} + +@test "a healthy pod on the node is captured beside the affected one" { + # The baseline is the point of the reference leg, and it only pays off in the + # SAME file as the affected pod: a reader comparing a suspicious route against + # a normal one should not have to go and find the normal one. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol' + echo 'cozy-cilium|cilium-xyz|10.0.0.5|node-a|True|Running|true|eol' + echo 'tenant|healthy|10.0.0.9|node-a|True|Running||eol' + exit 0 ;; + esac +done +case "$*" in + # A cni-server on the node, so the IP-specific legs actually run. Without it + # they are skipped and the baseline would be a header with no comparison in + # it -- which is the whole thing this leg is for. + *'app=kube-ovn-cni'*) echo 'cni-abc'; exit 0 ;; +esac +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 90 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + [ -f "$d/out/pod-tenant-test-wedged.txt" ] + grep -q 'POD tenant-test/wedged' "$d/out/pod-tenant-test-wedged.txt" + grep -q 'REFERENCE (Ready)' "$d/out/pod-tenant-test-wedged.txt" + grep -q 'POD tenant/healthy' "$d/out/pod-tenant-test-wedged.txt" + # And the baseline carries the evidence it exists to provide: the route and + # conntrack for the HEALTHY pod's address, not just a section header. A + # baseline without these compares nothing. + grep -q 'ip route get 10.0.0.9' "$d/out/pod-tenant-test-wedged.txt" + grep -q 'kernel conntrack for 10.0.0.9' "$d/out/pod-tenant-test-wedged.txt" + # The hostNetwork pod sorts first in the snapshot and must not have been + # taken: its podIP is the node's own address, the fingerprint the baseline + # exists to rule out. + if grep -q 'POD cozy-cilium/cilium-xyz' "$d/out/pod-tenant-test-wedged.txt"; then + echo "a hostNetwork pod was taken as the baseline:" + cat "$d/out/pod-tenant-test-wedged.txt" + exit 1 + fi + rm -rf "$d" +} + +@test "the baseline is captured at route-only scope, not the full one" { + # The baseline exists to show a normal route and conntrack beside a suspicious + # one. Cilium CT, the OVN port binding and the OVS ovn-installed flag describe + # how a pod was PROGRAMMED, and a pod chosen for being healthy is correctly + # programmed by definition -- they compare nothing and cost 105s of exec bound + # on the leg most likely to be cut. Dropping them is what keeps one affected + # pod on one node inside the 600s backstop, so the scope is load-bearing and + # not a preference. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol' + echo 'tenant|healthy|10.0.0.9|node-a|True|Running||eol' + exit 0 ;; + esac +done +case "$*" in + # Everything the full scope needs is present, so its absence from the + # baseline is a decision and not a missing dependency. + *'k8s-app=cilium'*) echo 'cilium-xyz'; exit 0 ;; + *'app=ovs'*) echo 'ovs-xyz'; exit 0 ;; + *'app=kube-ovn-cni'*) echo 'cni-abc'; exit 0 ;; + *'app=ovn-central'*) echo 'ovn-central-0'; exit 0 ;; +esac +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 120 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + f="$d/out/pod-tenant-test-wedged.txt" + [ -f "$f" ] + # Positive control: the affected pod gets the full scope, so the blocks exist + # and their absence below is scope and not a stub that answered nothing. + awk '/REFERENCE \(Ready\)/ { exit } { print }' "$f" | grep -q 'cilium-dbg bpf ct list global' + awk '/REFERENCE \(Ready\)/ { exit } { print }' "$f" | grep -q 'OVN Port_Binding' + # The baseline keeps what it is read for ... + awk '/REFERENCE \(Ready\)/,0' "$f" | grep -q 'ip route get 10.0.0.9' + awk '/REFERENCE \(Ready\)/,0' "$f" | grep -q 'kernel conntrack for 10.0.0.9' + # ... says in the artifact that the rest was a decision ... + awk '/REFERENCE \(Ready\)/,0' "$f" | grep -q 'scope: route and conntrack only' + awk '/REFERENCE \(Ready\)/,0' "$f" | grep -q 'absence below is a decision, not a lookup' + # ... and drops what compares nothing. Matched against SECTION HEADERS only: + # the marker line above names the very blocks it skips, so a bare substring + # search finds the explanation and calls it the block. + for section in 'cilium-dbg bpf ct list global' 'OVN Port_Binding' 'ovn-installed' 'Ready conditions'; do + if awk '/REFERENCE \(Ready\)/,0' "$f" | grep '^=== ' | grep -q "$section"; then + echo "the baseline was captured at full scope: section '$section' present after REFERENCE" + awk '/REFERENCE \(Ready\)/,0' "$f" | grep '^=== ' + exit 1 + fi + done + rm -rf "$d" +} + +@test "the baseline column is read from the cluster-wide snapshot" { + # The seventh column is what lets the baseline be chosen without a second + # read. Drop it from the jsonpath and every row is a six-field fragment that + # the NF guards reject -- no affected pods, no baseline, nothing captured. The + # stub answers only a request that asks for hostNetwork, so a jsonpath that + # stopped asking produces an empty list and this test goes red. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +case "$*" in + *'spec.hostNetwork'*) + echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol' + echo 'tenant|healthy|10.0.0.9|node-a|True|Running||eol' + exit 0 ;; +esac +for a in "$@"; do + case $a in + pods) exit 0 ;; + esac +done +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 90 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + [ -f "$d/out/pod-tenant-test-wedged.txt" ] + grep -q 'POD tenant/healthy' "$d/out/pod-tenant-test-wedged.txt" + rm -rf "$d" +} + +@test "two affected pods on one node share a single baseline capture" { + # The baseline depends on the node alone, and several affected pods on one + # node is what a wedged datapath looks like rather than an edge case. What + # must not repeat is the CAPTURE -- a second full per-pod capture of the same + # healthy pod, against a backstop the pod leg already strains. Counted by the + # baseline pod's own conditions read, which happens once per capture. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +case "$*" in + # The baseline runs at route-only scope, so its conditions read is gone -- + # count the route exec, which is the read that scope does make. + *'ip route get 10.0.0.9'*) echo ref-capture >>"$STUB_CALLS" ;; +esac +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged-a|10.0.0.1|node-a|False|Running||eol' + echo 'tenant-test|wedged-b|10.0.0.2|node-a|False|Running||eol' + echo 'tenant|healthy|10.0.0.9|node-a|True|Running||eol' + exit 0 ;; + esac +done +case "$*" in + *'app=kube-ovn-cni'*) echo 'cni-abc'; exit 0 ;; +esac +exit 0 +STUB + chmod +x "$d/bin/kubectl" + STUB_CALLS="$d/calls" PATH="$d/bin:$PATH" timeout 120 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + # Both pods carry the baseline: the saving must not cost one of them its copy. + grep -q 'POD tenant/healthy' "$d/out/pod-tenant-test-wedged-a.txt" + grep -q 'POD tenant/healthy' "$d/out/pod-tenant-test-wedged-b.txt" + n=$(grep -c ref-capture "$d/calls") + if [ "$n" -ne 1 ]; then + echo "the baseline was captured $n times for one node instead of once" + exit 1 + fi + rm -rf "$d" +} + +@test "the pod list jsonpath emits its columns in the order its readers index" { + # Every runtime test below answers the pod list from a stub that writes rows + # by hand, so none of them can observe the jsonpath's column ORDER: swap two + # fields in it and the suite stays green while the filters go on reading + # position 6 as phase and 7 as hostNetwork. Measured -- swapping + # `.status.phase` with `.spec.hostNetwork` passed 72 of 72. + # + # Against a cluster that swap makes pod_filter_affected compare a boolean + # against Succeeded/Failed and pod_first_ready compare a phase against + # "false", so the collector selects the wrong pods and says nothing. The + # positions are a contract between one jsonpath and three readers -- both awk + # filters and the `read` in the capture loop -- and no stub can hold it. + # + # Asserted as ORDER rather than as an exact string, so reformatting the + # jsonpath is free and reordering it is not. + # + # Each field carries its closing brace, and that is load-bearing rather than + # tidy: `.metadata.name` is a prefix of `.metadata.namespace`, so searching + # for it bare finds the namespace field and reports the two columns at one + # offset. The brace is what makes each search hit its own column. + line=$(grep -F '{range .items[*]}' "$SCRIPT" | grep -F 'spec.hostNetwork') + [ -n "$line" ] + prev=0 + for f in '.metadata.namespace}' '.metadata.name}' '.status.podIP}' '.spec.nodeName}' \ + 'type=="Ready")].status}' '.status.phase}' '.spec.hostNetwork}' '|eol"}'; do + cur=$(printf '%s\n' "$line" | awk -v s="$f" '{ print index($0, s) }') + # index() returns 0 for absent, so this one comparison catches a field that + # moved and a field that vanished. + if [ "$cur" -le "$prev" ]; then + echo "the pod list jsonpath no longer emits its columns in the order its readers assume" + echo "field '$f' is at offset $cur, expected after $prev" + echo "line: $line" + exit 1 + fi + prev=$cur + done +} + +@test "an affected pod gets the baseline from its own node" { + # The complement of the test above, and the half a shared cache can break + # without breaking that one: the cache is keyed by node, and a key that + # stopped telling nodes apart would serve every node the first node's + # answer. The test above counts captures and a single shared answer is + # exactly one capture, so it would stay green. + # + # What the reader would then get is a route and a conntrack table read on + # one kernel, filed under a heading naming a different node, with nothing + # in the artifact saying so -- a capture reporting what it never observed, + # which is the failure this collector exists to rule out. So the assertion + # is on WHICH healthy pod each file carries, per file, not on how many + # captures ran. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged-a|10.0.0.1|node-a|False|Running||eol' + echo 'tenant-test|wedged-b|10.0.0.2|node-b|False|Running||eol' + echo 'tenant|healthy-a|10.0.0.9|node-a|True|Running||eol' + echo 'tenant|healthy-b|10.0.0.8|node-b|True|Running||eol' + exit 0 ;; + esac +done +case "$*" in + *'app=kube-ovn-cni'*) echo 'cni-abc'; exit 0 ;; +esac +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 120 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + fa="$d/out/pod-tenant-test-wedged-a.txt" + fb="$d/out/pod-tenant-test-wedged-b.txt" + [ -f "$fa" ] + [ -f "$fb" ] + # Each file names its own node's healthy pod ... + grep -q 'POD tenant/healthy-a' "$fa" + grep -q 'POD tenant/healthy-b' "$fb" + # ... and carries that pod's address in the evidence, not just in a heading. + grep -q 'ip route get 10.0.0.9' "$fa" + grep -q 'ip route get 10.0.0.8' "$fb" + # ... and neither carries the other node's, which is what a node-agnostic + # cache key produces: both files served whichever node was reached first. + if grep -q 'POD tenant/healthy-b' "$fa" || grep -q 'POD tenant/healthy-a' "$fb"; then + echo "a baseline crossed nodes:" + grep -H 'POD tenant/healthy' "$fa" "$fb" + exit 1 + fi + rm -rf "$d" +} + +@test "a partially answered pod list is not called unanswered by the baseline" { + # The absence branch fires on "read failed AND no eligible pod", which is what + # a partial answer produces: rows arrive, none of them can serve -- the + # affected pod is not Ready, the hostNetwork pod is excluded -- and the list + # then dies mid-stream. Saying it never answered would assert what the run did + # not observe. It did not FINISH, which is true either way. + # + # With one snapshot this is the only way to reach that branch at all: a list + # that answered nothing leaves no affected pods, so no pod file is written. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol' + echo 'cozy-cilium|cilium-xyz|10.0.0.5|node-a|True|Running|true|eol' + exit 1 ;; + esac +done +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 90 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + [ -f "$d/out/pod-tenant-test-wedged.txt" ] + grep -q 'REFERENCE (Ready) pod on node=node-a: unknown' "$d/out/pod-tenant-test-wedged.txt" + grep -q 'the pod list did not finish' "$d/out/pod-tenant-test-wedged.txt" + rm -rf "$d" +} + +@test "a node whose snapshot holds no eligible pod says none found" { + # The other side of the same branch, and the reason it needs two wordings: a + # complete list with nothing eligible on the node is an answer, not a gap. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol' + echo 'cozy-cilium|cilium-xyz|10.0.0.5|node-a|True|Running|true|eol' + exit 0 ;; + esac +done +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 90 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + grep -q 'none found -- no baseline available' "$d/out/pod-tenant-test-wedged.txt" + if grep -q 'did not finish' "$d/out/pod-tenant-test-wedged.txt"; then + echo "a complete list was reported as unfinished:" + cat "$d/out/pod-tenant-test-wedged.txt" + exit 1 + fi + rm -rf "$d" +} + +@test "an affected pod is never offered as a healthy baseline" { + # Now true by construction rather than by exclusion: candidates come from the + # same rows the affected set came from, and one row cannot be both Ready and + # not-Ready. The test stays because the property is what matters, not the + # mechanism -- a future change that reads the node separately would reopen it. + d=$(mktemp -d) + mkdir -p "$d/bin" + cat >"$d/bin/kubectl" <<'STUB' +#!/bin/sh +for a in "$@"; do + case $a in + pods) + echo 'tenant-test|wedged-a|10.0.0.1|node-a|False|Running||eol' + echo 'tenant-test|wedged-b|10.0.0.2|node-a|False|Running||eol' + exit 0 ;; + esac +done +exit 0 +STUB + chmod +x "$d/bin/kubectl" + PATH="$d/bin:$PATH" timeout 120 "$SCRIPT" "$d/out" >"$d/log" 2>&1 || true + [ -f "$d/out/pod-tenant-test-wedged-a.txt" ] + [ -f "$d/out/pod-tenant-test-wedged-b.txt" ] + for f in "$d"/out/pod-tenant-test-wedged-a.txt "$d"/out/pod-tenant-test-wedged-b.txt; do + if grep 'REFERENCE' "$f" | grep -q 'tenant-test/wedged'; then + echo "a pod under investigation was used as the healthy baseline in $(basename "$f"):" + grep '^# POD' "$f" + exit 1 + fi + done + grep -q 'none found -- no baseline available' "$d/out/pod-tenant-test-wedged-b.txt" + rm -rf "$d" +} + @test "a memo hit reports the same answer as the miss that filled it" { # The status has to survive the memo. Every caller reads pod_on_node through a # command substitution, so a status kept in a variable dies with that subshell @@ -776,7 +1312,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; svc) echo 'tenant|web|LoadBalancer|192.0.2.10|80|30080|Cluster'; exit 0 ;; endpointslices) echo '10.0.0.1|node-a|tenant-test|wedged|true'; exit 0 ;; esac @@ -826,7 +1362,7 @@ STUB #!/bin/sh for a in "$@"; do case $a in - pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running'; exit 0 ;; + pods) echo 'tenant-test|wedged|10.0.0.1|node-a|False|Running||eol'; exit 0 ;; svc) echo 'tenant|web|LoadBalancer|192.0.2.10|80|30080|Cluster'; exit 0 ;; endpointslices) echo '10.0.0.1|node-a|tenant-test|wedged|true'; exit 0 ;; esac diff --git a/hack/e2e-capture-dataplane.sh b/hack/e2e-capture-dataplane.sh index e9bd8e208..df6a7ea63 100755 --- a/hack/e2e-capture-dataplane.sh +++ b/hack/e2e-capture-dataplane.sh @@ -9,7 +9,8 @@ # snapshot dir so the output lands in the uploaded cozyreport artifact, and both # wrap this script in a wall-clock backstop -- but not the same one (600s and # 300s respectively), so a change sized against one caller can still overrun the -# other. The MAX_LBS note below carries that arithmetic. +# other. The MAX_LBS and MAX_PODS notes below carry that arithmetic, one per +# fan-out cap. # # Why this exists: a recurrent install failure is a CNI host->local-pod # data-plane transient -- kubelet on a node reaches a *local* pod's @@ -39,6 +40,16 @@ # kernel-path misforward / missing route / wrong rule / unresolved neighbor # -- the actual mechanism behind host->local-pod "connection refused" on the # legacy-routing path (the crux of the transient). +# - Address attribution for a podIP that turns out to be a LOCAL address of +# the host netns instead of reachable via ovn0 (a route to nowhere-in- +# particular is not enough to name the owner): the same host netns above +# also gets `ip -o addr show` (every interface) and `ip route show table +# local`, and one Ready, Running, non-hostNetwork pod on the same node that +# has an address gets the route and conntrack half of the per-pod capture +# below as an in-run healthy-pod control, since this collector otherwise +# inspects NotReady pods only and had no same-window baseline. hostNetwork +# is excluded because its podIP IS the node's own address, so it would show +# the same local-route fingerprint as the fault for an unrelated reason. # - OVS/OVN on the node: `ovs-ofctl dump-flows br-int`, and the OVN # Port_Binding / Logical_Switch_Port for the pod (LSP name = . in # kube-ovn) plus a bounded `ovn-sbctl lflow-list`. @@ -94,9 +105,9 @@ # In-guest capture INSIDE the worker VMI (tenant `cilium-dbg bpf lb list` / # `tcpdump eth0`) is the one hop this cannot reach from the host; it is left # as a documented stretch at the call site (it needs a tenant -# kubeconfig/virtctl that neither caller of this diagnostic has). The host-side -# ANNOUNCER/ENDPOINT split is the deliverable and already localises the -# failing hop to host-cilium vs kube-ovn delivery. +# kubeconfig/virtctl that neither caller of this diagnostic has). The +# host-side ANNOUNCER/ENDPOINT split is the deliverable and already +# localises the failing hop to host-cilium vs kube-ovn delivery. # # Robustness contract (matches docs/agents/e2e-testing.md): pure diagnostics, # no retries, no behavior change, no traps. Every live capture is time-boxed, @@ -134,13 +145,24 @@ set -u # Service. Dropping it loses nothing a reader needs -- the read's own note # already says the list did not finish. # -# What a field count cannot catch: a cut inside the LAST field still delivers -# every separator, so `...|30080|Clus` counts seven and passes, and nothing -# in-band separates a truncated final value from a short one. Here the last -# column is display-only. In the two filters below it is a decision column -# (`phase`, `ready`), where a truncated value matches neither state being -# excluded and the row is kept -- over-capturing rather than dropping evidence, -# and the read's own cut-off note is what tells the reader the list ended early. +# What a field count cannot catch on its own: a cut inside the LAST field +# still delivers every separator, so `...|30080|Clus` counts seven and passes, +# and nothing in-band separates a truncated final value from a short one. Here +# the last column is display-only, so it costs nothing. +# `lb_first_ready_endpoint` decides on its own last column (`ready`) as a +# deny-list against the single value `false`, so a truncated value matches +# nothing it excludes and the row is kept -- over-capturing rather than +# dropping evidence, the safe direction for a diagnostic. +# +# The two pod filters do not rely on that reasoning at all. Their jsonpath +# ends in a constant `eol` column, and both require it present and intact, so +# a cut anywhere in a row either drops the field count or corrupts the +# sentinel. That matters for `pod_first_ready` specifically: it decides on +# `hostNetwork`, and a cut landing exactly on the preceding separator leaves +# that column EMPTY, which is byte-identical to the omitempty-false the filter +# must accept. No test of the value itself can tell those apart -- only a +# marker after it can. Either way the read's own cut-off note is what tells +# the reader the list ended early. lb_filter_services() { awk -F'|' 'NF == 7 && $3 == "LoadBalancer" && $4 != "" { print }' } @@ -247,19 +269,69 @@ lb_budget_ok() { } # pod_filter_affected: stdin = -# `namespace|name|podIP|nodeName|ready|phase` rows. Keep scheduled, non-terminal -# pods whose Ready condition is not True. podIP is deliberately NOT a gate: the -# cilium endpointManager leak this collector diagnoses can strand a pod before -# an IP is assigned, and the node-global Cilium/OVN state plus pod events remain -# useful in that state. IP-specific commands are gated later at their call sites. +# `namespace|name|podIP|nodeName|ready|phase|hostNetwork` rows. Keep scheduled, +# non-terminal pods whose Ready condition is not True. podIP is deliberately +# NOT a gate: the cilium endpointManager leak this collector diagnoses can +# strand a pod before an IP is assigned, and the node-global Cilium/OVN state +# plus pod events remain useful in that state. IP-specific commands are gated +# later at their call sites. # -# NF == 6 for the reason lb_filter_services carries NF == 7: a list cut -# mid-record leaves a fragment whose remaining columns still read as values, and -# a cut landing inside the nodeName column produces a half-parsed node this -# capture would then open a node-.txt for. Six fields per record is what -# the jsonpath emits. +# NF == 8 for the reason lb_filter_services carries its own count, though the +# counts differ because the jsonpaths do. A list cut mid-record leaves a +# fragment whose remaining columns still read as values, and a cut landing +# inside the nodeName column produces a half-parsed node this capture would then +# open a node-.txt for. Eight fields per record is what the jsonpath +# emits: six this filter reads, then hostNetwork for the baseline filter below, +# then a constant `eol` whose only job is to be the thing a truncation destroys. +# Both are checked here even though neither is read, because that is what makes +# a cut anywhere in the row detectable. pod_filter_affected() { - awk -F'|' 'NF == 6 && $4 != "" && $5 != "True" && $6 != "Succeeded" && $6 != "Failed"' + awk -F'|' 'NF == 8 && $8 == "eol" && $4 != "" && $5 != "True" && $6 != "Succeeded" && $6 != "Failed"' +} + +# pod_first_ready : stdin = the cluster-wide +# `namespace|name|podIP|nodeName|ready|phase|hostNetwork` rows, unfiltered. +# Emits `namespace|name|podIP` for the first pod ON that is Ready=True, +# Running, addressed and NOT hostNetwork -- the healthy-pod baseline for that +# node -- then stops, or nothing when the node has no eligible pod. +# +# It scopes itself rather than being handed a per-node list, and that is the +# design rather than a convenience. The rows are the same snapshot the affected +# set was computed from, so an affected pod cannot be selected here: +# pod_filter_affected keeps rows whose $5 is not True, this one requires $5 to +# be True, and a row cannot be both. Reading the node separately would need an +# exclusion list instead, because a pod that recovered between the two reads +# reports Ready in the later one and could be offered as the baseline for its +# own failure. +# +# hostNetwork is excluded because such a pod's podIP IS the node's own address, +# and cilium-agent / kube-ovn-cni / ovs-ovn run hostNetwork on every node. A +# baseline picked from those would resolve `ip route get ` to `local +# dev lo table local` for an entirely ordinary reason -- the exact fingerprint +# the address-attribution capture above exists to recognise -- so the +# comparison would defeat itself. A blank hostNetwork column counts as NOT +# hostNetwork, since the API omits the field when it is false. +# +# The test is written as an allow-list -- empty or the literal "false" -- rather +# than as `!= "true"`, which rejects a value cut mid-token: `tru` carries every +# field and passes every other gate, and because this filter stops at the row it +# takes, a deny-list would seat a hostNetwork pod as the baseline. That alone +# is not enough, though: a cut landing on the separator BEFORE hostNetwork +# leaves the column empty and indistinguishable from a genuine false. That case +# is caught by the constant `eol` column instead -- the row loses the sentinel +# with its tail. The two guards cover different cuts and neither replaces the +# other. +# +# An addressless pod is skipped for the same `exit`-at-first-match reason: a +# Ready pod that has no podIP yet would become the baseline and then carry none +# of the route or conntrack evidence the comparison exists for. +# +# NF == 8 and an intact `eol` for the reason the filters above carry their own +# counts: a list cut mid-record leaves a fragment whose remaining columns still +# read as values, and a fragment here would name a pod that does not exist as +# the baseline every reading of the affected pod gets compared against. +pod_first_ready() { + awk -F'|' -v node="$1" 'NF == 8 && $8 == "eol" && $4 == node && $3 != "" && $5 == "True" && $6 == "Running" && ($7 == "" || $7 == "false") { print $1 "|" $2 "|" $3; exit }' } # How a cutoff should be described, mirroring prevlog_cutoff_desc in the sibling @@ -315,8 +387,52 @@ CILIUM_NS="${COZY_CILIUM_NS:-cozy-cilium}" KUBEOVN_NS="${COZY_KUBEOVN_NS:-cozy-kubeovn}" # Cap how many pods we inspect so a fully-wedged cluster cannot explode the # runtime; the per-command timeouts and the call-site wall-clock wrapper are the -# other two bounds. Node-global captures are deduped per node, so the effective -# work is closer to (#affected-nodes) than (#affected-pods). +# other two bounds. Node-global captures are deduped per node, and so is the +# healthy-pod baseline, so the total is (#affected-pods) per-pod captures plus +# (#affected-nodes) baselines and node-global captures -- not two per pod. +# +# The header asks for this arithmetic against the tighter caller rather than +# only the roomier one, so here it is. Every term counts the OUTER bound of +# each call and adds the bounded lookups that call makes; a nested inner +# timeout is not a second wait. The terms are the whole path before +# capture_lb_datapath, not only the per-pod ones: the preamble is 82s of the +# total, and a sum that omits it reports a margin roomy enough for another +# collector where the real one has no room at all. +# - preamble, once per run: 82s -- the cluster pod list at 28s+2s, the +# ovn-central lookup at 20s+2s, and the 30s ovn-sbctl lflow-list; +# - a node capture bounds at 288s: 222s of execs (the `cilium-dbg monitor` +# leg counts its outer 12s, not the inner 8s it wraps) plus three 22s +# pod_on_node lookups, memo misses the first time a node is seen; +# - an affected pod's capture bounds at 179s: 135s of execs plus two 20s +# reads with their 2s kill graces; +# - the baseline adds 30s per affected node: it runs at route-only scope, so +# the route and the conntrack and nothing else, and it issues no read of +# its own. +# +# One affected pod on one node therefore bounds at 579s, inside the 600s the +# cozytest.sh trap allows and still past the 300s the Chainsaw catch shares +# with its snapshot leg. That margin is 21 seconds, so the next collector +# added to this path does not fit and the number has to be recomputed rather +# than assumed. Read the 21s as what this path leaves the rest of the run, not +# as headroom the run has: capture_lb_datapath executes unconditionally after +# it and inside the same backstop, and the MAX_LBS note above says that section +# bounds work rather than wall clock, with each heavy capture costing tens of +# seconds. +# +# The 579s holds only WHILE the lookup memo exists. When mktemp has failed and +# _POD_MEMO is empty every pod_on_node is live again, each capture pays for +# its own, and the same case bounds at 667s -- outside both backstops. The +# guarantee is conditional and this is the condition. +# +# Route-only scope is what buys it. At full scope the baseline costs 179s +# instead of 30s, the single-pod case bounds at 728s, and it would lose a +# backstop it clears at the merge base (519s there). The three blocks that +# scope drops describe how a pod was programmed, and a pod chosen for being +# healthy is programmed correctly by definition -- they compare nothing. +# +# What the numbers do say is that this cap counts pods while the ceiling is +# measured in seconds, and past one affected pod the two stop meeting: raising +# MAX_PODS buys nothing the envelope can pay for. MAX_PODS="${COZY_DATAPLANE_MAX_PODS:-12}" # LoadBalancer-datapath section tunables (see the header block and the @@ -411,9 +527,18 @@ log() { # Failed pods (completed Jobs, hook pods) are Ready!=True too but are not either # symptom, so they are excluded to keep the MAX_PODS budget on actual wedges. # jsonpath keeps this dependency-free (no jq / go-template reassignment). +# +# hostNetwork rides along on this one read even though only the healthy-pod +# baseline consults it, and only for nodes that turn out to have an affected +# pod. Asking per node instead would be a second read of the same objects, and +# the two answers could then disagree: a pod that recovered in between reports +# Ready in the later one and could be offered as the baseline for its own +# failure. One snapshot makes that impossible rather than guarded -- the +# affected set and the baseline candidates are the same rows -- and the extra +# column costs one boolean per pod on a list this script already takes. # shellcheck disable=SC2086 # empty DP_LIST_BOUND must vanish, not become "" _pods_raw=$($DP_LIST_BOUND kubectl get pods -A \ - -o jsonpath='{range .items[*]}{.metadata.namespace}{"|"}{.metadata.name}{"|"}{.status.podIP}{"|"}{.spec.nodeName}{"|"}{.status.conditions[?(@.type=="Ready")].status}{"|"}{.status.phase}{"\n"}{end}' \ + -o jsonpath='{range .items[*]}{.metadata.namespace}{"|"}{.metadata.name}{"|"}{.status.podIP}{"|"}{.spec.nodeName}{"|"}{.status.conditions[?(@.type=="Ready")].status}{"|"}{.status.phase}{"|"}{.spec.hostNetwork}{"|eol"}{"\n"}{end}' \ 2>"${DP_ERR:-/dev/null}") _pods_rc=$? if [ "$_pods_rc" -ne 0 ]; then @@ -497,6 +622,11 @@ mark_node() { _SEEN_NODES="$_SEEN_NODES$1 "; } # be paid for again in the sections that follow. Empty when mktemp fails, which # only costs the memo. _POD_MEMO=$(mktemp "${TMPDIR:-/tmp}/dataplane-podmemo.XXXXXX" 2>/dev/null) || _POD_MEMO="" +# Cache for the per-node healthy-pod baseline, one file per node, for the same +# reason and with the same subshell constraint as the memo above. Empty when +# mktemp fails, which costs the saving and not the evidence: the baseline is +# then captured per pod, the way it was before the cache existed. +_REF_CACHE=$(mktemp -d "${TMPDIR:-/tmp}/dataplane-refcache.XXXXXX" 2>/dev/null) || _REF_CACHE="" # The stored row carries the read's status beside its value. A hit that dropped # it would hand a later caller an empty answer with no way to tell "there is no # such pod" from "the read never said", which is the distinction the callers @@ -613,6 +743,23 @@ capture_node() { echo "=== host netns: ip addr show ovn0 ===" timeout 15 kubectl exec -n "$KUBEOVN_NS" "$cni" -c cni-server -- \ ip addr show ovn0 2>&1 || true + + echo + echo "=== host netns: ip -o addr show, all interfaces (address attribution) ===" + # ovn0 alone cannot name the owner when a podIP turns out to be a LOCAL + # address instead: the interface actually holding it might be anything on + # the host (see the header block). This is the same info scoped to every + # interface, not just ovn0. + timeout 15 kubectl exec -n "$KUBEOVN_NS" "$cni" -c cni-server -- \ + ip -o addr show 2>&1 || true + + echo + echo "=== host netns: ip route show table local (address attribution) ===" + # A podIP resolving to `local dev lo table local` here is the + # fingerprint from the header block: it wins over the ovn0 route in + # `main` because table local is consulted first (see `ip rule` above). + timeout 15 kubectl exec -n "$KUBEOVN_NS" "$cni" -c cni-server -- \ + ip route show table local 2>&1 || true else echo if [ "$_cni_ok" -eq 0 ]; then @@ -624,6 +771,210 @@ capture_node() { } >> "$nf" 2>&1 || true } +# capture_pod_dataplane