The shell translator rewrites test bodies and loses Bats reporting.
The unit lane needs complete TAP results and JUnit reports without
changing how live-cluster suites run.
Sourced production helpers still need a dash pass, and unit test bodies
need nounset without exporting shell options into subprocesses.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Test-level EXIT handlers replace the handler Bats uses for TAP output,
so a failed assertion can disappear from the reported test results.
Cleanup must preserve those failures before the unit runner changes.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
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>
Multus-backed NetworkAttachmentDefinitions delegate to the upstream
containernetworking reference plugins (bridge, macvlan, ipvlan, ...),
and nothing guarantees a node has them in the directory Multus
searches. On k3s they live under /var/lib/rancher/k3s/data/cni while
Multus and Cilium install into /opt/cni/bin, so a bridge-type NAD fails
there with `failed to find plugin "bridge" in path [/opt/cni/bin]`.
Bundle 14 of the release's plugins into the multus image and add an
install-cni-plugins init container that installs them into the host
/opt/cni/bin, mirroring the existing install-multus-binary container.
loopback, dhcp, tap and dummy are left out; the Dockerfile records why.
Gate it on a new chart value, stageCniPlugins, resolved per bundle and
on for both. Talos bundles only bridge, firewall, flannel, host-local,
loopback and portmap, so the other ten are missing there exactly as on
generic Linux; the four that overlap are rewritten with the same
upstream version, v1.9.1 on both sides, rather than an older one. A
marker in the image Dockerfile records the Talos release that pin was
compared against, and a test holds it to the release the platform
ships. multus therefore moves out of system.common-packages into the
two variant branches, the way linstor already does for talos.enabled.
networking.stageCniPlugins overrides the variant default in either
direction.
Where staging is on the install is unconditional: a plugin is replaced
on every pod recreation, not only when the destination is missing. A
destination that is already a directory is reported and left alone,
since replacing it is not something this can do safely. Turning the
value off afterwards does not undo it; nothing here removes or
restores a plugin.
The container is not privileged and takes no mount propagation. It
runs in the spc_t SELinux domain, without which an enforcing node
refuses the write. Each plugin is installed under a temporary name and
swapped in with a rename: /opt/cni/bin is shared with cilium and
kube-ovn. The plugin tarball is checksummed before extraction.
A plugin that cannot be installed is reported on stderr and to
/dev/termination-log, and the container still exits 0: Multus stays the
node's primary CNI and nothing removes its config, so an init container
that never completes would leave the shim with no daemon behind it.
The manifest change lives in customize-deployment.patch so it survives
make update.
BREAKING CHANGE: enabling stageCniPlugins replaces reference CNI
plugins in /opt/cni/bin on existing nodes, Talos included -- four of the
names it writes are ones the Talos node image also carries. Opt out
before upgrading: declining afterwards does not restore what was
replaced.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Myasnikov Daniil