Repository navigation

Commit 1b52832

authored

fix(e2e): stop the data-plane capture reporting what it never observed (#3672)

## What this PR does Six fixes to the e2e host->pod data-plane collector and the two call sites that invoke it, all of one class: a diagnostic that states a fact it never observed. Every one of them makes a failed run look healthier than it was, which is the worst direction for a tool that only ever runs on an already-failed test. - A list cut mid-record parsed as a valid record. Three `awk -F'|'` row filters selected by field value and never checked the field count, so an IP cut mid-octet still looked like an IP and a cut inside a nodeName still looked like a scheduled pod. The capture then probed an address the cluster never had and opened a per-node file for a node that does not exist. Each jsonpath emits a fixed field count per record, so a shorter row is a fragment by construction. - Five distinct paths ran zero probes and all reported "reachable, skipped": no node to probe from, a Service with no port, a probe node with no `kube-ovn-cni` pod, a host netns with no `nc`/`curl`/`wget`, and an exec that could not run. An image without a probe client alone would stamp every LoadBalancer in the cluster reachable and skip the whole heavy capture. They now report unprobed, each with a reason the run can actually support. - Both EndpointSlice reads discarded stderr and were read by emptiness alone, so a refused read wrote the same `<none>` a Service with no endpoints writes. They now carry their status. That change armed a landmine the silence had hidden: the target port was read with `{.items[0]...}`, and client-go's `evalArray` has no `allowMissingKeys` escape, so an endpoint-less Service is a hard error rather than an empty answer. The read now asks with `{.items[*]...}`. - `pod_on_node` turns its node argument into `--field-selector spec.nodeName=<node>`, so an empty node asks for pods whose nodeName is empty, i.e. unscheduled ones. Both tcpdump legs could hand a pending pod to a capture stamped ANNOUNCER or ENDPOINT. Each leg is now guarded and says in the artifact that it was skipped. - Both callers wrapped the collector in a wall-clock backstop and discarded the result, while the previous-logs leg beside each of them reports "INCOMPLETE (exit N)". A truncated `dataplane/` directory was indistinguishable from a complete one that found little. - The file's own header credited one caller where two exist, under different backstops (600s and 300s), so a change sized against the named one can overrun the omitted one. Diagnostics only. No retries, no cluster mutation, no change to any test's pass/fail outcome: the statuses that are now read are used to print a line and nothing else. Every behavioural change is pinned by a test in the existing bats surface, verified red against the committed tree before the fix. The two suites run 53 and 5 tests. Known limits and deliberate omissions, named so the next reader does not have to rediscover them: - A field count cannot catch a cut inside a record's LAST field, since every separator has already arrived. The comment on the first filter says so and says where it lands for each of the three. The MetalLB speaker loop is left without a count for the same reason: its records carry two fields, and `speaker-1|nod` is indistinguishable from a genuinely short node name, so a count there would reject nothing while reading as a closed gap. - The backstop lines report any non-zero status as `INCOMPLETE (exit N)`. Exit 127 (no `timeout` on PATH) means nothing ran, so "kept what landed" overstates it. Left alone deliberately: the same wording ships on the previous-logs and crust-gather legs beside them, and splitting only the two new ones would create the asymmetry this PR exists to remove. All four want the same treatment together. - When a probe's cni-server lookup fails once and then succeeds while no probe client is present, the reason line names the lookup for all three attempts. The named cause did occur, so nothing false reaches the artifact, but it is narrower than what happened. - `hack/capture-dataplane.bats` now needs `yq` for one test, and unlike three sibling suites it does not declare that at the top of the file. The fan-out caps are sized in units of work while the backstops are wall-clock, and the worst case exceeds both by multiples. That predates this branch and is unchanged by it, but the new INCOMPLETE line is what will make it visible in an artifact for the first time. Tracked separately in #3671. relates to #3658 #3660 #3654 #3657 #3663 #3649 ### Screenshots  ### Downstream repositories  - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note  ```release-note fix(e2e): the host->pod data-plane capture no longer reports absence or reachability it never observed -- truncated list records are dropped instead of parsed, a LoadBalancer no probe reached is recorded as unprobed rather than reachable, an unread EndpointSlice backend is distinguished from an absent one, a tcpdump leg whose node was never identified is skipped instead of aimed at an unscheduled pod, and both callers report a capture their backstop cut short ```  ## Summary by CodeRabbit * **Bug Fixes** * Improved data-plane capture reliability and reporting for timeouts, failed or partial reads, and missing collectors. * Prevented incomplete records and unknown probe results from being treated as valid or reachable. * Avoided capturing from unintended nodes when node information is unavailable. * Preserved partial diagnostic output while clearly reporting incomplete captures. * **Tests** * Expanded coverage for capture outcomes, artifact accuracy, timeout handling, and guarded node selection.

2 parents 9a0c8c7 + c696154 commit 1b52832Copy full SHA for 1b52832

5 files changed

hack

Comments

(0)

File tree

0 commit comments