mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-07 04:23:50 +09:00
Aleksei Sviridkin
1b52832f37
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## 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
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [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
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```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
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->