mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 14:02:56 +09:00
ff3294e02df49491b17455e95f3ad3967bded13b
102
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
2b3c163d15 |
fix(e2e): size tenant workers by resources, not by u1.large
Sizing the workers with u1.large doubled their memory along with the vCPUs, and four 8Gi workers do not fit on the sandbox beside the rest of the suite: the second worker of each cluster stayed Pending with Insufficient memory on all three nodes, so the node group could never reach two Ready nodes and node-join failed for a reason that has nothing to do with the CPU ceiling it was meant to test. Setting resources.cpu and resources.memory keeps the two vCPUs at the memory the workers had before. The chart drops the instancetype from the VM when a group sets both, and the values schema still requires the field, so it stays in the values as the sizing fallback it now is. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
3b49be07fa |
test(e2e): give tenant workers two vCPUs
The tenant worker boots Talos and unpacks the kubelet image on a single vCPU whose CFS quota also covers the QEMU emulator and IO threads, and measurements on both quiet and loaded runs show the compute container pinned at 93-98% of that one-core ceiling for the whole node-Ready budget. The join deadline is then decided entirely by how much CPU work the boot happens to need, and under parallel e2e load it does not fit. Doubling the worker to u1.large raises the ceiling instead of the weight, which measurements showed changes nothing since the workers never lose a scheduling fight. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
527d4ea00f |
test(e2e): measure tenant worker CPU as a rate, and the sandbox nodes under it
Two instruments for the node-join failure the kubernetes-* suites keep hitting. Neither changes what the suites assert. The worker CPU capture now reads container_cpu_usage_seconds_total beside the CFS counters, and reads them twice a fixed interval apart instead of once. The throttled counters say a container met its ceiling and cannot say how much CPU it got, which are the two candidate explanations for a stalled worker: a guest held at a one-core ceiling, and a guest whose vCPU thread is rarely scheduled at all. Both leave throttled periods behind. Every counter involved accumulates from container start, so a single reading is an average over the whole uptime and no rate at all, and the profile under investigation is a guest that freezes for tens of seconds and then runs flat out: the average comes out low, the instantaneous figure comes out at the cap, and only the second separates it from a guest that is simply capped. The sandbox nodes are a new subject. The worker captures describe a cgroup, so a guest that is runnable and never scheduled looks from there like a guest that asked for nothing. Whether the sandbox node itself was given its turn is recorded only in the steal column of its own /proc/stat: no kubelet endpoint carries it, and crust-gather cannot reach it because PodSecurity refuses its node-shell Pod here, so the read goes over the Talos API, which is already how the report reaches those nodes for dmesg. The rows are captured verbatim with their column legend beside them rather than reduced to a percentage, because /proc/stat has no header, steal and guest sit next to each other, and guest is large on a node running VMs and already counted inside user. Both subjects are read on either side of one shared wait, so the pair pays it once. The wait is not the interval either subject's counters span, though: each pass walks its own nodes and the other subject's pass falls between a subject's two readings, and on the run this exists for those passes are the slow part. Each capture therefore stamps both ends of its read, so a rate comes from subtracting stamps rather than from assuming the knob, and the width of those brackets is the uncertainty. That wait is a measurement interval rather than a stand-in for a condition nobody wrote a wait for, so the e2e conventions carve it out by name instead of marking it as a fixed timeout to be replaced. The pair is gated below the serial console rather than above it. Its ceiling is sixteen bounded reads plus the interval, which is most of the phase budget, and admission gates only when a collector may start, so above the console a run that hit those bounds would leave the console never started rather than merely cut short. The console is the only capture that survives a worker which never reached apid, which is the shape this failure usually takes. That overrun is inside the term the budget derivation already allows for one late admission, and a guard holds the pair's ceiling against it. No collector here may leave an empty directory, since an empty directory reads as a healthy node: a missing binary, a missing talosconfig, a listing that never answered, a node with no address and a read that was cut off each land as a named note or a COLLECTION-FAILED.txt. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
3dd142a718 |
fix(e2e): capture both ends of the worker byte path on node-join failure (#3804)
## What this PR does A tenant worker that misses the node-Ready deadline while its kubelet image pull crawls leaves two mechanisms behind one symptom, and the failure artifact could not tell them apart: either the link is slow, or the pull keeps restarting and discarding what it fetched. From inside the guest both are a progress line that barely moves. This adds the host-side counters that separate them, and the guest-side reads that explain a kubelet which never got as far as pulling at all. **Host side.** A per-worker network capture reads the eight `container_network_*` families from the same kubelet cAdvisor endpoint the CPU throttling capture already uses. cAdvisor builds them from the interface counters inside the Pod's own network namespace and publishes one row per interface, so what arrived at the worker is counted there in full, including the bytes a restart threw away, set against the progress the guest reported, that separates a pull which kept restarting from a slow link. Which row answers is not obvious, and getting it wrong inverts the reading, so the collector states it rather than leaving it to the reader. A bridge-bound worker presents four interfaces in its Pod netns: the one still named `eth0` is a dummy that holds the Pod IP so the kubelet's check passes and carries no traffic, so it reports zero, and it is the row a reader reaches for first, by name, where zero reads as a worker that received nothing. The number that answers is the renamed NIC's receive; the bridge counts the same bytes again, and the tap's counters are named from the host's end. The filter deliberately carries no interface predicate, so all four reach the artifact and can be told apart. Two further properties of the family fail by capturing nothing rather than loudly, so both are stated where the filter is: the dropped-packet series are spelled `packets_dropped` and there is no `*_drops_total`, and every row carries an empty container label because cAdvisor publishes them on the Pod sandbox, which means a filter borrowed from the CPU counters beside it matches nothing at all. The fixture's row shape and values are real kubelet output with the identifying labels rewritten. Its four interface names are derived rather than observed: no virt-launcher Pod was available to sample, so they are read from the name generators in the KubeVirt version this repo ships, and the counters attached to them illustrate direction rather than magnitude. **Guest side.** The Talos capture gains the service state machine and the link table. Talos creates a service's log buffer when its runner opens the log writer, as it starts the process, so a kubelet still waiting on its volumes, on time sync, or on the container runtime reporting healthy never reached its runner and has no log at all, and `talosctl logs kubelet` then fails naming a log that was never registered, which is true about the log and silent about the service. `talosctl services` answers with the state and the condition being waited on, in one call for every service, so the runtime a kubelet is blocked behind arrives in the same file. The link table is read as YAML because MTU lives in the `LinkStatus` spec and not among the resource's printed columns, and it is the value that decides whether a path-MTU reading of a stalled transfer survives. Both new reads are made with the short-lived `os:reader` credential the capture already mints; neither needs a wider role. **A worker that answered nothing now says so once, for the worker.** Each read already carried its own error and exit code, which is accurate per file and adds up to a directory indistinguishable from a worker that simply had little to report. ### What this does not do This collects evidence; it fixes nothing and is not expected to change any suite's outcome. The comparison it enables is not symmetric either, and the source says so: the host side reports bytes on an interface, while the guest side reports an image pull's own progress, which is unpacking rather than arrival. They bound each other; they are not two measurements of one quantity. ### Cost The diagnostics phase budget drops from 8m to 7m. That is forced rather than chosen: the inequality that keeps the tenant crust-gather snapshot reachable had ten seconds of slack, and the two added guest reads cost sixty across the minimum worker pool. A shorter budget declines more, and what it declines first is what is gated last: the mirror capture, whose subject is partly recoverable elsewhere, and the image-cache re-probe, which has no substitute and was already first to go. The new network capture also spends a listing and one read per node ahead of the guest captures, so those reach their gate with less headroom than before. All of it is priced where the number lives, as a ceiling the read bounds impose rather than a duration measured on a live cluster, and a collector the budget declines says so in the log rather than going quiet. The remaining slack is ten seconds, which is worth knowing before the next collector is added rather than after: it buys no bounded read at any bound this file uses, so adding one means re-deriving the budget again. ### Testing `hack/run-kubernetes-network-counters_test.bats` is new (15 tests). The node-join, Talos-diagnostics and CPU-throttle suites gain coverage for the phase ordering, the per-worker marker boundary, the spend-order comments in both Chainsaw suites, and the enumeration of collectors that survive a missing `timeout`. Every claim the two Chainsaw comment blocks make about spend order is derived from the source rather than restated, so a collector added without a line there fails. The two cAdvisor captures now run one shared body. They previously carried near-identical copies, and the copies cost more than duplication normally does: the block-level bound audit pins each collector by something only it produces, and two collectors issuing identical reads left neither with anything of its own, so removing one from the audit went unnoticed. Each is now pinned by the report directory it writes. Verified with `hack/cozytest.sh` across eleven suites, checking that the number of tests executed matches the number declared, because an exit code answers "did anything fail", never "did anything run". Each behavioural change was confirmed by mutating the source and observing the intended test fail. ### Screenshots No UI changes. ### Downstream repositories Walked the trigger map file by file against the diff. All seven changed files live under `hack/`, and the change adds one file and modifies six; no repository file is moved, renamed or deleted. The `ccp` entry is the only one that keys on `hack/` at all, and it keys on `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` as anchors for `make generate`; none of those is touched, and no make target changes behaviour. Nothing here is a package, a chart value, an `ApplicationDefinition`, a CRD, a release asset, a namespace, a variant, or a node prerequisite, so no other entry applies. - [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 NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added worker network-counter diagnostics with interface-level and cumulative traffic data. * Expanded Talos diagnostics with service-state and link-state information. * **Bug Fixes** * Improved reporting for partial captures, timeouts, unavailable data, and zero-traffic workers. * Clarified failure reporting and fallback messages when timeout controls are unavailable. * **Documentation** * Updated diagnostic timing and collector documentation for the seven-minute budget and new metrics. * **Tests** * Added comprehensive coverage for network counters, filtering, truncation, and expanded Talos diagnostic scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
e441edc2aa |
test(e2e): stop the mongodb backup suite before the restore leg
A to-copy restore replays the source cluster's system users into the target, so the target's own generated credentials stop authenticating and the harness cannot read the sentinel back out of the restored copy. The suite has never passed for that reason, holding the in-tree e2e run red on every branch that selects it. Drive run-all.sh with SKIP_RESTORE=1 so the suite still proves the backup half end to end, and pin the skip by asserting the skip branch's own log line: exit zero alone is also what a completed round-trip returns, so without that assertion the step could not tell a skipped flow from a full one, nor notice SKIP_RESTORE failing to reach the harness. The to-copy manifest and the example README both stated that CI covers the restore, which stops being true here, so both now say where coverage ends. The asserted log line is annotated at its source, since rewording it would red the suite with a bare match failure. Coverage returns by dropping SKIP_RESTORE together with the assertion on that line, once the credential handling behind the restore is fixed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
|
61edf33ef4 |
fix(e2e): capture both ends of the worker byte path on node-join failure
A tenant worker that misses the node-Ready deadline while its kubelet image pull crawls leaves two mechanisms behind one symptom, and the artifact could not tell them apart. Either the link is slow, or the pull keeps restarting and discarding what it fetched; from inside the guest both are a progress line that barely moves. Per-worker network counters, read from the kubelet's cAdvisor endpoint the CPU throttling capture already uses, count every byte that crossed into the Pod including the ones a restart threw away, so the gap between that and the progress the guest reported separates the two. Which row carries it is not obvious and getting it wrong inverts the reading, so the collector states it. cAdvisor publishes one row per interface in the Pod netns, and a bridge-bound worker has four: the one still named eth0 is a dummy holding the Pod IP, carries no traffic, and reports zero -- the row a reader reaches for first, where zero reads as a worker that received nothing. The renamed NIC's receive is the number that answers; the bridge counts the same bytes again and the tap is named from the host's end. No interface predicate is applied, so all four reach the artifact and can be told apart. Two further properties of the family fail by capturing nothing rather than loudly: the dropped-packet series are spelled packets_dropped, and every row carries an empty container label because cAdvisor publishes them on the Pod sandbox, so a filter borrowed from the CPU counters beside it matches nothing at all. Both are stated where the filter is. The fixture's rows are real kubelet output relabelled; its interface names are read from the KubeVirt version this repo ships rather than sampled, since no virt-launcher Pod was available. The guest side gains the service state machine and the link table. Talos creates a service's log buffer when its runner opens the log writer, as it starts the process, so a kubelet still waiting on its volumes, on time sync, or on the container runtime reporting healthy never reached its runner, has no log at all, and the capture recorded an rpc error naming a log that was never registered: true about the log, silent about the service, while the state and the awaited condition were the finding. The link table is read as yaml because MTU lives in the spec rather than among the printed columns, and it is the value that decides whether a path-MTU reading of a stalled transfer survives. Both carry a tighter bound than the two reads beside them, being single tabular RPCs rather than streams, and the walk they sit in is the term the phase budget sizes itself against. A worker whose reads all failed now says so once for the worker. Each log already carried its own error and exit code, which is accurate per file and adds up to a directory indistinguishable from a worker that had little to report. The phase budget drops from 8m to 7m to pay for the two added guest reads, because the inequality keeping the tenant snapshot reachable had ten seconds of slack. A shorter budget declines more, and what it declines first is what is gated last: the mirror capture, whose subject is partly recoverable from its own request counts and the host snapshot, and the image-cache re-probe, which has no substitute and was already the first to go. The second cost lands on the guest captures, which are gated after the new counters and reach their gate with both the shortened budget and a listing plus a read per node already spent -- up to 100s at the read bound and the three-node cap, a ceiling those numbers impose rather than a duration measured on a live cluster. Sharing that read would mean the two captures becoming one walk, which is left out for scope rather than merit: the phase gate is a deadline rather than a per-collector allowance, so one walk admitted at the first of the two gates would cost less and collect more. The cost is stated rather than netted away, and the source says which way the argument runs. Both captures run one shared body, which also settles what the two copies had already broken once: the audit that catches an unbounded read pins each collector by something only it produces, and two collectors issuing byte-identical reads left neither with anything of its own, so stubbing one out of the audit went unnoticed. It is pinned by the report directory each writes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
829edfadb3 |
test(e2e): warm the ghcr.io mirror before the tenant suites need it
The in-sandbox pull-through cache fetches a blob only when a client asks for it, so the first tenant worker's kubelet pull is a live fetch from ghcr.io. That leg is what misses the node-Ready budget: measured at 40-165 KB/s for a 61 MiB image, with two blob streams dying mid-transfer after 7-8 MiB on an upstream HTTP/2 reset. Talos holds the kubelet service in PreFunc until the pull completes, so the node never registers, and the tenant cilium HelmRelease then fails for want of a schedulable node rather than for any fault of its own. Fill the cache from a Job started in the same install step that deploys the mirror, tens of minutes before the kubernetes suites run. The Job waits for the mirror to serve, then asks it for each kubelet tag's index, walks the manifests it gets back, and discards the blob bodies; distribution populates its blobstore on the way past. It warms one platform per tag -- the one the workers run -- picked out of the index by record, which keeps the walk a grep. Walking both platforms was a true addition to the same throttled leg: arm64 is bytes nothing in the sandbox pulls, and the digest ordering put it first for both tags, so on the slow runs this exists for the deadline expired with the second tag still cold. It warms the two minors the suites select, not the whole version map. The map would be ~300 MiB at the one-platform walk this ships (~61 MiB per tag), three fifths of it for minors no suite can resolve, spent on the same throttled egress the rest of the install is using. The tag list still comes from the chart's map rather than a copy, ordered the way the suites resolve it, so the set is theirs without restating their expression. The readiness wait is a wall clock, not a per-attempt count. Cilium answers a Service with no ready endpoints by rejecting, so a probe costs nothing when the mirror is absent and the full timeout when it is merely slow; a constant was wrong in both directions across two attempts at picking one. Tags are refused by character class before the pattern is applied, so a value cannot pass by having a valid first line: grep matches per line and the pattern's anchor is end-of-line, not end-of-input. The Job exits non-zero on any miss, so backoffLimit is a real bound and a warm-up that reached nothing cannot report success. It names its own requests, because the node-join diagnostic counts kubelet GETs in the access log to tell a worker that pulled through the mirror from one that fell back to public ghcr.io. Naming them is only half of that. The same diagnostic now reports the Job's status and result and the Job outlives the suites, because excluding the warm-up without reporting it would leave a red run unable to distinguish a warm cache from a warm-up that never started. That counter also moved onto the access-line shape: registry:2.8.3 writes three lines carrying the path per request, so matching the path counted one pull as three. It reads the repository from the same variable the warm-up does, so a rename cannot leave the diagnostic counting zero while the workers pull under the new name. GET and HEAD both count: containerd resolves a manifest with a HEAD first, so a GET-anchored count reports a pull that died before its first GET as no request at all. The failure path also prints how long the mirror took to answer the worker. A warm cache and a cold one are otherwise indistinguishable in the artifacts -- the Job reports success either way, because the proxy stores the blob in the background and a completed GET is a different event from a stored blob -- and server-side duration separates them by orders of magnitude. It filters the log in the shell that read it: a single argv element is capped at 131072 bytes on Linux regardless of ARG_MAX, and probe traffic carries this log past that within about twenty minutes of mirror uptime, so handing it to a helper would empty the section on every run worth reading. Nothing is printed when the log could not be read at all, because a heading with no lines under it reads as a finding about the workers. Measured against a local pull-through registry using the script the builder emits: 32m47s cold, 1.1s warm for the same tags, and the blobs it fetches are the same digests the failing runs stalled on. A warm-up that fails, times out or never starts does not fail the step, and the suites behave as they did before it existed. It is not free, though: it pulls across the same throttled egress the install is using, which is why the warm set is two tags and not five. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
9dc49de1c9 |
fix(apps): make the post-delete cleanup hooks report failure (#3593)
## What this PR does The qdrant and harbor post-delete cleanup hooks could not fail. Every delete was guarded by `|| echo "WARNING: ..." >&2`, neither script set `-e`, and both ended on an unconditional completion message, so the container exited 0 on every path. Helm read that as a successful hook, `helm.sh/hook-delete-policy: hook-succeeded` removed the Job together with the pod that held the warning, and an uninstall that reclaimed nothing looked exactly like one that had nothing to reclaim. Written up in #3566. Both hooks now count failures and set the exit code at the end. `set -e` would abort at the first failed delete and skip the rest of the sweep, which is the opposite of what is wanted: every remaining object should still be attempted and the outcome reported once. Already-gone objects stay a success, since every delete keeps `--ignore-not-found`. Each failure message names the object it left behind. `packages/apps/mariadb` already uses this shape. Both hooks also consumed `kubectl get` output directly: harbor by iterating `$(kubectl get ...)`, qdrant by piping it into `grep`, whose status the pipeline then reports. A failed listing arrived as an empty list, and the sweep reported success without having looked. Each listing is now captured before it is used. #3566 attributes that shape to harbor's two loops alone. qdrant has it too: `pvcs=$(kubectl get pvc ... | grep -E ... || true)` reports grep's status, so a `kubectl get` that failed left `$pvcs` empty and the hook announced "no data PVCs found; nothing to delete". Both instances are fixed here. ### The cost A hook that exits non-zero keeps its Job and its pods, because `hook-succeeded` does not fire for it. That is the point, the pods hold the log, but it means a failed uninstall now leaves objects behind where it previously left none. Nothing collects them on its own: `before-hook-creation` only fires when the hook runs again, and a `post-delete` hook runs only on an uninstall, so they stay until a release of that name is installed and uninstalled again, or until the namespace goes. The release itself is not stranded. Helm appends the post-delete hook error to its error list instead of returning on it, marks the release uninstalled, purges it from storage anyway, and only then reports `uninstallation completed with N error(s)`. The failure surfaces once and the app still goes away. Both Jobs get one retry instead of the Kubernetes default of six, and instead of none. Every delete is idempotent, so a second attempt re-issues an API call rather than re-running any logic, and one short backoff separates a transient apiserver error from a real one. These failures become visible for the first time with this change, and a signal that fires on noise stops being read, including on the day it is right. Six is the far end of the same mistake: ten minutes of backoff across seven pods outlasts the hook timeout, so Helm reports that timeout instead of the hook's own error. `packages/apps/clickhouse` and `packages/apps/mariadb` already exit non-zero from their cleanup hooks and set no retry limit, so a real failure in either spends the full backoff and reaches the operator as a timeout rather than as the error the hook wrote. Adding the exit code without the cap reproduces that. One limitation is deliberate. harbor's ACME sweep cannot tell "this kind is not served" from "I could not look". If cert-manager is torn down before the release, `kubectl get challenge` fails and the uninstall is reported failed with nothing to sweep. cert-manager is a hard dependency of the bundle harbor ships in, so the window is narrow, and a CRD-existence probe would add a second silent-skip path of its own: it would swallow a discovery failure the way this change stops swallowing a listing failure. ### Tests `hack/app-cleanup-hooks_test.bats` renders each hook's script out of its chart and runs it against a fake `kubectl`, because the property under test is the container's exit code and matching against manifest text cannot establish it. There are eight failure paths across the two hooks, six in harbor and two in qdrant, and each has a case that goes red when that one path alone is reverted to a swallowed warning. Also covered: a clean delete, and an object that is already gone in both its shapes, an empty listing and a `NotFound` on the delete itself. The fake `kubectl` models `--ignore-not-found` instead of exiting 0 unconditionally, so dropping that flag from a hook turns the already-gone cases red. Message assertions are anchored on the hook's own `ERROR: ` prefix, because the fake names the object in its own diagnostic too, and an unanchored match is satisfied by the fixture. Two manifest fields are pinned in each chart's existing helm-unittest suite, where manifest fields belong. The retry cap is one. The other is `helm.sh/hook-delete-policy`: adding `hook-failed` to that annotation hands the failed Job straight back to Helm to delete, which undoes the whole change. I checked that before adding the assert, and the reversal left both suites fully green. The qdrant Chainsaw suite explained its post-delete pod collector by quoting the guard string this change removes, and concluded that Helm always collects the pod away. That conclusion inverts here, so the note is rewritten, along with the sentence in the harbor suite that points at it. ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. It touches two app chart templates and adds one file under `hack/`. No package added, renamed or removed, no `values.schema.json`, no `values.yaml` default, no version enum, no `ApplicationDefinition`, no namespace, no node prerequisite, and neither `hack/package.mk` nor `hack/update-crd.sh`. - [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(apps): the qdrant and harbor post-delete cleanup hooks now exit non-zero when a delete or a listing fails, instead of reporting success on every path. `helm uninstall` reports the failure rather than returning clean, and the hook Job and its pods stay in the namespace so the log survives to be read. Nothing collects them on its own: they remain until a release of that name is installed and uninstalled again, or until the namespace is removed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved Harbor and Qdrant cleanup reliability by detecting listing and deletion failures. - Cleanup jobs now retry once and report failures with a non-zero status. - Cleanup continues processing remaining resources after individual failures. - Successful cleanup remains safe and repeatable, including when resources are already missing. - Harbor cleanup remains limited to resources associated with the configured Harbor host. - **Tests** - Added comprehensive coverage for successful, repeated, partial, and failed cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
09dc7f47fb |
feat(backups): add a native MongoDB (psmdb) backup-strategy driver (#3562)
## What this PR does Adds a native MongoDB (Percona Server for MongoDB / psmdb) backup-strategy driver so `apps.cozystack.io/MongoDB` reaches feature parity with the CNPG, MariaDB and ClickHouse native drivers — logical backups, restore into a differently-named instance, and point-in-time recovery — all through the unified `BackupJob` / `RestoreJob` / `BackupClass` / `Plan` interface (closes #3545). The driver orchestrates the operator's `PerconaServerMongoDBBackup` / `PerconaServerMongoDBRestore` CRDs: - **BackupJob**: creates a one-shot `PerconaServerMongoDBBackup`, polls `status.state`, and persists the S3 destination + storage descriptor (by reference — Secret name only, never a raw credential) onto the Cozystack `Backup` artifact. - **RestoreJob**: drives `PerconaServerMongoDBRestore` via `backupSource` (read back from the source backup status, with a snapshot fallback for a reaped operator CR) so the same artifact restores in-place **or** into a differently-named instance. PITR is exposed as `spec.options.recoveryTime` (RFC3339), matching the CNPG driver's option exactly; unrecognised `spec.options` keys are surfaced as a `UnknownRestoreOption` Warning event rather than silently dropped. Because psmdb resolves the backup storage by name against the cluster's `spec.backup.storages`, the driver names the chart's storage (`s3-storage`) via the `cozy-default-mongodb` Strategy rather than injecting it, and surfaces a clear `Ready=False` precondition when a target has backups disabled instead of hanging until the deadline. **Wiring** - `api/backups/strategy/v1alpha1`: `MongoDB` strategy type + generated CRD/deepcopy. - `internal/backupcontroller`: `psmdbstrategy_controller` + `psmdbtypes` / `mongodbapp` partial-schema mirrors; BackupJob/RestoreJob dispatch, supported-kinds, cleanup-on-delete branch, and scheme registration. - `backupstrategy-controller`: `cozy-default` MongoDB route, `cozy-default-mongodb` Strategy CR, and `psmdb.percona.com` RBAC (get/list/watch, +create on backups/restores, no delete — the driver does not own the S3 archive). - `apps/mongodb`: a `backup.insecureSkipTLSVerify` value (psmdb's s3 storage has no CA-bundle field) so backups can target Cozystack's self-signed in-cluster seaweedfs. Default `false`, fully backward-compatible; regenerated schema/README/RD. **Tests + e2e** - Unit tests for the driver: applicationRef validation, ensure-idempotency (backup + restore), the backups-enabled/storage precondition gate, `backupSource` resolution (live-preferred / snapshot fallback / no-destination), snapshot round-trip, the producer-seam (`createMongoDBBackupArtifact`) driver-metadata, the cleanup-on-delete no-Velero-fall-through guard, and `recoveryTime` → psmdb-UTC mapping + unknown-option reporting. - `examples/backups/mongodb/` + a `mongodb-2-backup-roundtrip` Chainsaw suite that drives `run-all.sh` end-to-end (a sentinel document round-trips through S3 into a restored copy while the source stays untouched), per `docs/agents/e2e-testing.md`. - `docs/operations/backup-classes.md` updated (new route, the app-owns-the-storage model, and the `recoveryTime` PITR usage). Verified end-to-end on a live dev cluster against the platform-default `cozy-backups` bucket: backup via `cozy-default` reached the bucket and the Cozystack `Backup` went `Ready`, and a to-copy `RestoreJob` restored the sentinel into a differently-named instance with the source left intact. ### Screenshots N/A — no UI changes (controller, chart values, CRD, docs and tests only). ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` file by file against the diff: - **cozystack/website** — the only app-facing change is `packages/apps/mongodb` (a new `backup.insecureSkipTLSVerify` value + regenerated `README.md`). The managed-app reference page is regenerated from the package `README.md` by the release bot for apps already in the website `Makefile` (mongodb is one), so it refreshes automatically on the next tag — no manual follow-up. `docs/operations/backup-classes.md` is not a website-mirrored doc in the trigger map. - **cozystack/terraform-provider-cozystack** — the one candidate trigger ("a field added to an app's `values.schema.json`") lands inside the mongodb `backup` block, which the provider's `internal/provider/mongodb_schema.go` explicitly states it does **not** manage ("The deprecated backup block, the bootstrap restore block, and detailed shardingConfig are not managed"). No enum, default, `release.prefix`, Secret/Service name, or hand-typed CRD (`Plan`, `RestoreJob`) changed; the new `strategy.backups.cozystack.io/MongoDB` CRD is opaque to the provider. Not affected. - All other repositories — no matching trigger (no `hack/` layout or make-target change, no namespace rename, no `ApplicationDefinition` semantics change, no `packages/core/platform` values, no telemetry metric or `cozy-proxy` label rename). - [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 feat(backups): add a native MongoDB (psmdb) backup-strategy driver — logical backup to the cozy-default BackupClass, restore into a differently-named instance, and point-in-time recovery via spec.options.recoveryTime ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added MongoDB backup and restore support, including scheduled and on-demand backups, restores to new or existing clusters, and point-in-time recovery. * Added configurable S3 TLS certificate verification for MongoDB backups, defaulting to secure verification. * Added MongoDB support to the default backup configuration. * **Documentation** * Added MongoDB backup and restore examples, workflows, and operational guidance. * **Tests** * Added automated coverage for MongoDB backup, restore, cleanup, and end-to-end scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
b8a5b9ba1d |
test(e2e): capture worker CPU throttling counters on node-join failure
When a tenant worker misses the node-Ready deadline, the sandbox node it runs on is routinely at half its capacity with a clean kernel log. In that state "the host starved the VM" and "the VM was held at its own CFS ceiling" produce identical evidence: node-level utilisation reports what the node used, never what the container was allowed. The CFS counters are the only place the difference is recorded, and nothing in the tree collected them for the tenant workers, so every diagnosis of this failure so far had to be inferred from outside. Read them from the kubelet's cAdvisor endpoint on the node hosting each worker. Five series carry the whole answer and cAdvisor publishes all of them per container, so nothing is computed here: the scheduled and throttled period counts, the time spent throttled, and the quota and period that form the ceiling. The throttled counters alone say a container hit some ceiling, not which one, and a VM capped at one core and a VM capped at eight are the same number without the quota. Absence of the quota series is itself a reading rather than a gap. cAdvisor emits it only when the quota is non-zero, so a container reporting the period and no quota is one running uncapped - which is one of the two answers this collector exists to distinguish, arriving without being derived. Reading the kubelet rather than the container matters beyond convenience. A reader running inside the container shares the cgroup it measures, so it slows down in proportion to how much the answer matters and would need a budget sized for the pathological case; the kubelet is outside that cgroup. It also drops any dependency on what the container image ships and on how the host lays cgroups out. The namespace is not a worker filter and is not used as one: it also holds the tenant control plane, whose apiserver and etcd carry CPU limits of their own and can be genuinely throttled. The Pod name carries the second half of the filter, so a throttled apiserver cannot answer under a heading that says worker. The walk is over nodes rather than Pods, because the endpoint is per node and one read covers every worker it hosts. It is gated on the diagnostics phase budget, and placed first among the five gated collectors: the budget declines whatever has not started when it runs out, its own ceiling is four bounded reads rather than the minutes the four after it can spend, and it is the only one whose question has no other answer in the tree. That order is a trade, stated here because it is a decision rather than a detail - it puts the guest serial console one place further from the budget, and that capture is the only one describing a worker which never reached apid. Both reads take the block's own read-budget knob rather than a literal equal to it, so lowering that knob lowers these too. Each keeps its own exit status rather than routing through the shared helper, which always returns zero by design: this collector has to tell "the kubelet was not read" from "the kubelet answered and carried no worker", and only the status separates them. The stream is captured whole and filtered after, since a pipeline reports its last command's status and would otherwise hand that decision to grep. Every way a read can come up short is named rather than left as zero bytes: a kubelet that was not read, one that answered with nothing for the namespace, a stream cut short, and exit 124 - which cannot distinguish this collector timing out from the read exiting 124 on its own - each get their own line, and each says whether an error log exists to explain it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
e9411d6b8c |
fix(apps): make the post-delete cleanup hooks report failure
The qdrant and harbor post-delete hooks guarded every delete with `|| echo "WARNING: ..." >&2`, set no `-e`, and ended on an unconditional completion message, so the container exited 0 on every path. Helm read that as a successful hook and the hook-succeeded delete policy removed the Job together with the pod that carried the warning, leaving an uninstall that reclaimed nothing indistinguishable from one that had nothing to reclaim. Count the failures instead and set the exit code at the end, so a failed delete does not skip the deletes that follow it. An object that is already gone stays a success: every delete keeps --ignore-not-found, which exits 0 in that case. This is the shape packages/apps/mariadb already uses for the same problem. Both hooks also consumed the output of `kubectl get` directly -- harbor by iterating it, qdrant by piping it into grep, whose status the pipeline then reports. A listing that failed therefore arrived as an empty list and the sweep was skipped rather than reported. Each listing is now captured before it is used, so a listing that failed and one that returned nothing are different events. A hook that exits non-zero keeps its Job and pods, which is the point: they hold the log. Give the Job one retry rather than the Kubernetes default of six or none at all. Every delete is idempotent, so a second attempt re-issues an API call rather than re-running any logic, and one short backoff is what separates a transient apiserver error from a real one -- these failures become visible for the first time here, and a signal that fires on noise stops being read, including on the day it is right. Six is the far end of the same mistake: ten minutes of backoff across seven pods outlasts the hook timeout, and Helm reports that timeout instead of the hook's own error. The qdrant Chainsaw suite explained its post-delete pod collector by quoting the guard string removed here and concluding the pod is always collected away. That is now backwards: a failed cleanup is exactly when the pod survives and the collector is the evidence. Rewrite the note, and the sentence in the harbor suite pointing at it. hack/app-cleanup-hooks_test.bats renders both scripts and runs them against a fake kubectl. There are eight failure paths across the two hooks -- six in harbor, two in qdrant -- and each has a case that goes red when that one path alone is reverted to a swallowed warning. Message assertions are anchored on the hook's own `ERROR: ` prefix, because the fake kubectl names the object in its own diagnostic too and an unanchored match is satisfied by the fixture. The helm-unittest suites pin the retry limit and the hook delete policy: adding hook-failed to that annotation hands the Job back to Helm to delete on failure and destroys the evidence this change exists to keep, and until these asserts that reversal left both suites green. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
516be4abfe |
fix(cozystack-basics): normalize tenant hostname VAP comparisons with lowerAscii (#3208)
## What this PR does The Ingress, Gateway, and Route hostname `ValidatingAdmissionPolicy` objects compare tenant hostnames against the tenant apex with case-sensitive CEL. The resource-side host is always lowercase — the apiserver's hostname validation permits nothing else — but both apex operands can carry uppercase: the `namespace.cozystack.io/host` label value, which Kubernetes permits to carry uppercase and which the tenant chart writes verbatim from `tenant.spec.host`, and the templated `_cluster.root-host`, a free-form platform value that nothing validates for case. A mixed-case apex label wrongly denies a tenant its own legitimate lowercase in-apex hostname. An uppercase `_cluster.root-host` is the dangerous direction: the Ingress outside-root exclusion compares a lowercase host against the uppercase literal, never matches, so the exclusion stops excluding and the policy admits any non-wildcard host — every non-wildcard platform-apex name a tenant asks for, including a hostname belonging to another tenant and the root apex itself. Only the anti-wildcard gate still holds. This normalizes both operands of every hostname comparison with `.lowerAscii()` (host, apex label, and the templated root-host literal) across all three policies, so the match follows DNS case semantics. There is no behavior change in the default all-lowercase configuration. The Gateway policy's fail-open on an absent or empty host label is deliberately left as it was: that is the platform's one documented fail-open exception in `docs/security/threat-model.md`, so changing the posture is a security-model decision rather than a side effect of normalizing case. Its current shape is pinned by a test. Test coverage: - helm-unittest pins each comparison as a whole operand pair — equality and `endsWith`, across the rule-host and tls-host branches — so dropping the normalization from any single operand is caught. A pattern matching a lone `.lowerAscii()` would not: the apex appears four times in the Ingress expression, so stripping one operand still leaves three matches. - A rendering case with `_cluster.root-host: Example.ORG` pins the outside-root escape shut; it is red pre-fix. - e2e probes set a mixed-case apex on an isolated throwaway namespace and assert both halves of the match: a lowercase in-apex HTTPRoute and Ingress are still admitted (red pre-fix, green post-fix), while a foreign apex and a hostname ending with the apex without a dot boundary are still denied. Vacuous-pass guards re-read the persisted label and assert the probe apex really is mixed-case. Normalizing the apex at the write site — the tenant namespace template and `_cluster.root-host` — is tracked separately in #3328. It would not remove the need for this change: the host label is settable out-of-band by a trusted caller, so these policies must not assume the apex arrives normalized. Closes #3205. ### Screenshots N/A (no UI changes). ### Release note ```release-note fix(cozystack-basics): tenant hostname admission policies now compare against the tenant apex and the platform root apex case-insensitively. This closes a bypass where an uppercase `_cluster.root-host` made the Ingress policy's outside-root exclusion stop matching, so the policy admitted any non-wildcard host — including a hostname under the platform apex belonging to another tenant, and the root apex itself. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Hostname validation now consistently matches case-insensitively across Gateway, Ingress, HTTPRoute, and TLSRoute. * Mixed-case tenant and apex hostnames are accepted when they meet the intended domain boundaries. * Foreign domains and prefix-adjacent hostnames remain correctly rejected; validation behaves as documented when the tenant host is empty. * **Tests** * Expanded unit and e2e coverage for case-insensitive hostname rendering and boundary rejections across the supported resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
dff56aa998 |
feat(backups): add a native MongoDB (psmdb) backup-strategy driver
Adds a MongoDB (Percona Server for MongoDB / psmdb) backup-strategy driver so `apps.cozystack.io/MongoDB` reaches feature parity with the CNPG, MariaDB and ClickHouse native drivers — logical backups, restore-to-differently-named instances, and point-in-time recovery through the unified BackupJob/RestoreJob/BackupClass/Plan interface (closes #3545). The driver orchestrates the operator's PerconaServerMongoDBBackup / PerconaServerMongoDBRestore CRDs: - BackupJob: creates a one-shot PerconaServerMongoDBBackup, polls status.state, and persists the S3 destination + storage descriptor (by-reference, no credentials) onto the Cozystack Backup artifact. - RestoreJob: drives PerconaServerMongoDBRestore via backupSource (read back from the source backup status, snapshot fallback) so the same artifact restores in-place or into a differently-named instance; PITR via spec.options.pitr (date/latest). Because psmdb resolves the backup storage by name against the cluster's spec.backup.storages, the driver names the chart's storage (s3-storage) via the cozy-default-mongodb Strategy rather than injecting it, and surfaces a clear Ready=False precondition when a target has backups disabled. Wiring: - api/backups/strategy/v1alpha1: MongoDB strategy type + generated CRD/deepcopy. - internal/backupcontroller: psmdbstrategy_controller + psmdbtypes/mongodbapp partial-schema mirrors; dispatch, supported-kinds and scheme registration. - backupstrategy-controller: cozy-default MongoDB route, cozy-default-mongodb Strategy CR, and psmdb.percona.com RBAC. - apps/mongodb: backup.insecureSkipTLSVerify knob (psmdb s3 storage has no CA-bundle field) so backups can target Cozystack's self-signed seaweedfs. Tests + e2e: - unit tests for the driver (validation, ensure-idempotency, precondition, backupSource resolution, snapshot round-trip, PITR option parsing). - examples/backups/mongodb + a mongodb-2-backup-roundtrip Chainsaw suite that drives run-all.sh end-to-end (sentinel document round-trips through S3 into a restored copy while the source stays untouched), per docs/agents/e2e-testing.md. - docs/operations/backup-classes.md updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
d0990299ed |
feat(clickhouse): let the backup sidecar trust a private S3 CA, unblocking the backup e2e round-trip (#3385)
## What this PR does Gives the ClickHouse `clickhouse-backup` sidecar a way to trust an S3 endpoint whose certificate is signed by a private CA, and uses that to make the ClickHouse backup/restore e2e round-trip actually run in CI. This actualizes #2600, whose feature code and example scripts have since landed on `main` independently, and whose remaining contribution — the e2e test — was still in the retired BATS format. #2600 can be closed once this merges. ### 1. `backup.endpointCA` on the ClickHouse chart The sidecar had **no TLS/CA surface at all**: bucket coordinates and credentials arrive as `S3_*` env, and nothing let it verify an endpoint signed by a private CA. Cozystack's own in-cluster SeaweedFS is exactly that case — it serves `:8333` behind the self-signed "SeaweedFS CA" — so a ClickHouse release could only ever back up to a publicly-trusted endpoint. MariaDB (`storage.s3.tls.caSecretKeyRef`) and Postgres (barman `endpointCA`) have had a CA field all along; this closes the gap for ClickHouse. `backup.endpointCA` takes `{name, key}`. When set, the chart mounts that Secret into the sidecar and points `SSL_CERT_DIR` at it. Empty — the default — renders no env, no mount and no volume, so existing releases are byte-for-byte unchanged. **Why `SSL_CERT_DIR` and not `AWS_CA_BUNDLE`.** Both are honoured by `altinity/clickhouse-backup:2.7.4`; I measured them against a self-signed endpoint and public AWS S3 in turn. Go reads `SSL_CERT_DIR` *in addition to* the system bundle, so both endpoints verify. `AWS_CA_BUNDLE` *replaces* the SDK's trust pool: the private endpoint verifies, and public S3 then fails `x509: certificate signed by unknown authority` — it would silently break a release that also talks to a public endpoint. `S3_DISABLE_CERT_VERIFICATION` was rejected outright, since it drops verification rather than trusting one more CA. ### 2. The e2e suite, and what it was really testing Two Tests added to the existing `hack/e2e-chainsaw/clickhouse/` suite (in name order, mirroring the `etcd`/`mariadb`/`postgres` shape — see the layout rationale below for why they live here rather than in a new directory): - **`clickhouse-1-backup-contracts`** (CI, `tenant-test`, ~98s): a backup-enabled ClickHouse asserted against the *running* cluster — the `clickhouse-backup` sidecar on the `ch-backup-api` `:7171` port, and the `<release>-backup-api-auth` Secret whose keys the sidecar's `API_USERNAME`/`API_PASSWORD` actually resolve `valueFrom`. Each is a silent break if a chart bump renames it. Needs no object store. - **`clickhouse-2-backup-roundtrip`** (CI, `tenant-root`): `Bucket` → `Altinity` strategy + `BackupClass` → source ClickHouse with a sentinel row → `BackupJob` → in-place `RestoreJob` → to-copy `RestoreJob` into a second instance, driving `examples/backups/clickhouse/run-all.sh` so test and docs cannot drift. The round-trip was originally gated behind `CLICKHOUSE_E2E_S3_ROUNDTRIP` and **skipped in CI** — its `PASS (1.01s)` meant the gate fired, not that a backup happened. `backup.endpointCA` removes the reason for the gate, so the suite now adopts the same three preconditions mariadb/postgres rely on: run in `tenant-root` (an isolated tenant's Cilium egress allowlist blocks its Pods from reaching `tenant-root`'s SeaweedFS), target the in-cluster endpoint instead of the `Bucket`'s external ingress URL (an unroutable placeholder in CI), and trust its certificate via the copied CA. It also pre-cleans with `cleanup.sh`, because `tenant-root` is shared and a leftover `Succeeded` `BackupJob` would satisfy the harness's wait without a backup ever running. Both backup Tests live in the existing `hack/e2e-chainsaw/clickhouse/` suite rather than a new suite directory, so no selector mapping is added: `hack/select-e2e.sh` already derives `clickhouse` both from `packages/apps/clickhouse/**` (the `*-application` rule) and from `examples/backups/clickhouse/**` (the harness path), and both paths therefore select the suite that actually runs the harness. That alignment is the reason for the layout — a separate `clickhouse-backup/` directory would have made an edit to the harness select a suite that cannot exercise it. `hack/select-e2e.sh` and `hack/select-install.sh` are otherwise untouched (the only change is a comment recording that invariant), and the existing `backup example harness edit selects its app suite` case in `hack/select-e2e_test.bats` covers the mapping. ### Verification - `helm unittest` — 17 pass, including 5 new `endpointCA` cases: the unset default renders nothing, the env/mount/volume wiring, a custom key still projected as `ca.crt`, composition with `useSystemBucket`, and `backup.enabled=false` renders nothing. - The CA-trust behaviour was measured against the real image, not assumed: without a CA knob the sidecar reports `x509: certificate signed by unknown authority`; with `SSL_CERT_DIR` the handshake succeeds. - `hack/select-e2e_test.bats` (16) and `hack/select-install_test.bats` (17) pass; `select-install.sh --validate` reports graph OK. - `make generate` is idempotent, so the pre-commit job's `git diff --exit-code` stays clean. - The contracts Test passed in CI on earlier runs of this branch. The round-trip only became CI-live with this work, so it still needs a green E2E run before merge — the branch has since been rebased onto current `main` to get one. ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff, file by file. The load-bearing change is the new `backup.endpointCA` field in `packages/apps/clickhouse/values.yaml` / `values.schema.json`; everything else is `hack/e2e-chainsaw/`, `examples/backups/`, one docs paragraph, and regenerated artifacts, none of which appear in the map. - [ ] No downstream repository is affected by this change - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/18 — "add, remove or rename a field in an app's `values.schema.json`" reaches the provider's hand-maintained schema, model and expand/flatten pair, so the new nested object is not surfaced until it is added there. Opened as an issue rather than a PR because the natural moment is alongside that repo's next `api/apps/v1alpha1` pin bump, which is a maintainer call. Not ticked, with reasons rather than silence: **cozystack/website** — the trigger is *adding, renaming or removing a package*, and this adds a field to an existing one. `clickhouse` is already in the website `Makefile`'s app list and its reference page is generated from this package's `README.md`, so the regenerated `backup.endpointCA` rows are carried by the existing release-tag automation with no manual PR. **cozystack/external-apps-example** — vendors `hack/package.mk` and `update-crd.sh`, neither of which this PR touches. **cozystack/ansible-cozystack**, **ccp**, **talm**, **cozyhr**, **cozy-proxy**, **cozystack-telemetry-server**, **examples** — no installer values, `hack/` layout, namespace, variant, node-prerequisite, annotation, label or metric in their trigger rows is touched here. ### Release note ```release-note feat(clickhouse): add `backup.endpointCA` so the clickhouse-backup sidecar can verify an S3 endpoint whose certificate is signed by a private CA, such as Cozystack's in-cluster SeaweedFS. Leave it unset for publicly-trusted endpoints; existing releases are unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional private S3 endpoint CA support for ClickHouse backup/restore sidecars via `backup.endpointCA`, including configurable Secret name and CA bundle key (default `ca.crt`). * Sidecars mount the CA and extend trust while leaving public endpoints unchanged. * **Documentation** * Updated ClickHouse chart docs, operational backup guidance, and backup/restore examples to cover CA setup, overrides, and cleanup. * **Tests** * Added helm-unittest and end-to-end coverage validating CA rendering/mounting behavior and ClickHouse backup/restore workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
ba15b3817a |
fix(cozystack-basics): normalize tenant hostname VAP comparisons with lowerAscii
The Ingress, Gateway, and Route hostname ValidatingAdmissionPolicies compare tenant hostnames against the tenant apex with case-sensitive CEL. The resource-side host is already lowercase — the apiserver's hostname validation permits nothing else — but the apex operand is the namespace.cozystack.io/host label value, which Kubernetes permits to carry uppercase, and the Ingress policy additionally compares against the templated root host, a free-form platform value that nothing validates for case. A mixed-case apex label wrongly denies a tenant its own legitimate lowercase in-apex hostname. An uppercase root host is the dangerous direction: the Ingress outside-root exclusion compares a lowercase host against the uppercase literal, never matches, so the exclusion stops excluding and the policy degenerates into admitting any non-wildcard host — every non-wildcard platform-apex name a tenant asks for, including the root apex itself. Only the anti-wildcard gate still holds. Wrap both operands of every comparison in .lowerAscii() so the match follows DNS case semantics — no behavior change for the all-lowercase default. Each policy reads a label it does not own: the namespace host label is settable out-of-band by a trusted caller, so none of them may assume the apex arrives normalized. The Gateway policy admits any listener hostname when the host label is absent or empty, unlike its two siblings. That is the platform's one documented fail-open exception, recorded in the threat model together with its compensating control, so changing the posture is a security-model decision rather than a side effect of normalizing case. This leaves the behavior untouched and pins its current shape in the tests. Covered by helm-unittest assertions pinning each comparison as a whole operand pair, so dropping the normalization from any single operand is caught; a rendering case that pins the uppercase-root-host escape shut; and e2e probes that set a mixed-case apex on an isolated namespace and assert both halves of the match — a lowercase in-apex HTTPRoute and Ingress are still admitted, while a foreign apex and a hostname that ends with the apex without a dot boundary are still denied. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
9d4bcd8d66 |
fix(e2e): wire the ghcr.io mirror into the tenant install and node-join flow
Deploy the pull-through mirror from its own install step rather than folding it into the Talos image-cache step, whose early return on a missing values key would otherwise skip it silently. Build the tenant CR spec.talos block from both the OS image cache and the registry mirror, each included only when its in-sandbox backend is up. On a node-join failure dump the mirror's state alongside the Talos image cache, under the same phase gate as its neighbours so it cannot starve the guest captures of budget. Record the mirror's legs in the step's budget inventory. talos_spec_block is covered by a bats suite over all four mirror-up and mirror-down combinations, splicing the result under spec: and reading it back with yq so a block indented into the wrong parent fails, and the install step is pinned to exclude the Cilium policy from its pre-Cilium apply. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
1c6ee7c047 |
fix(e2e): harden the ghcr.io mirror resolver and bound its diagnostics
resolve_ghcr_mirror_endpoint decides once, for the whole shared sandbox, whether tenant workers are pointed at the in-sandbox mirror, so a misclassified transient failure would disable it for the rest of the run. Cache only stable answers: a genuine API NotFound and a definite Available=False; never a transient query error, a missing kubectl, or a failed egress-allow apply. Read readiness from the Deployment's own Available condition when the rollout watch exits non-zero, and commit the endpoint only once the egress allow is in place, surfacing the apply failure reason rather than a generic blip. The node-join diagnostic counts kubelet-image requests across the whole access log rather than a fixed tail, and every read it makes is wall-clock bounded by the caller's COZY_DIAG_READ_TIMEOUT so no single read can hold the failure path open. The bats suite exercises every resolver branch and the diagnostic's filtering, with a kubectl stub that reports errors on stderr as the real binary does and a timeout mock so the bounded reads stay reachable under test. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
58910c0edc |
feat(kubernetes): route tenant worker image pulls through a registry mirror
Add a talos.registryMirrors passthrough to the kubernetes and kubernetes-nodes
charts that renders into the worker TalosConfigTemplate machine.registries.mirrors,
so an operator can point a registry such as ghcr.io at an in-cluster pull-through
mirror and a worker boot no longer depends on live public egress. Default {}
keeps the rendered worker reconcile Job byte-identical to before, content-hash
Job name included. Free-form mirror values are shell-escaped for the unquoted
apply heredoc, covered by an execution-level bats test.
Also add the e2e ghcr-mirror (registry:2 pull-through plus a Cilium egress
policy) and wire it into the kubernetes chainsaw suites, so CI exercises the
mirror path and takes the flaky public ghcr.io kubelet-image pull off live
egress.
Refs #3513 (addresses the egress variant of the tenant-worker node-join
failures; the dominant variant is CI-runner CPU starvation, tracked in the
issue).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
|
5400f165e3 |
fix(e2e): keep a failed mirror lookup from pinning the public factory
resolve_talos_image_factory_url asked whether the in-cluster Talos image mirror exists with a plain `kubectl get deploy`, where a connection refused, an Unauthorized and a real absence all arrive as the same non-zero exit. It then cached that reading for the whole suite, so one unlucky moment sent every tenant worker to the public Image Factory over live egress -- the dependency the mirror exists to remove -- while the run still looked ordinary. Ask with --ignore-not-found -o name instead: a real absence answers exit 0 with no output, which leaves a non-zero exit meaning only that the question failed, and re-ask a couple of times before giving up. Only the empty answer turns the suite toward the public factory. An unanswered lookup warns, falls back for its own caller alone, and is not written to the decision file, so the next tenant test asks again rather than inheriting a guess. Each attempt is bounded twice, and the two bounds are deliberately not equal. kubectl defaults --request-timeout to 0, so a connection that establishes and then stalls has no deadline of its own; the flag alone is not a wall-clock bound either, because kubectl retries discovery several times before giving up. Measured against a stub apiserver, the three ways this call fails behave differently: a refused port answers in under a second and names the refusal, one that stalls mid-TLS gets kubectl's own handshake deadline and still produces text, and a blackholed address produces nothing until some deadline expires. With both bounds at the same value the expiry that arrives first is always the wall-clock kill, so --request-timeout can never fire and that last case stays silent. The wrapper therefore sits ten seconds above it: the request deadline ends the attempt and kubectl says why, and the wrapper remains the backstop for a client that overruns its own deadline. Where timeout is not installed the read runs unwrapped rather than through it: its exit 127 would otherwise count as an attempt that did not answer, three times over, and a missing local binary would pin the whole suite to the public factory while every message blamed the apiserver. kubectl's stderr is left alone, because it is the only thing separating refused from unauthorized from stalled. It is still not guaranteed, since a kill can land before the client has said anything, so the lookup also names the last attempt's exit status before giving up, in the vocabulary the diagnostic reads use for 124 and 137. The diagnostic path asks the same question and keeps its own answer to it. That one gives up after a single attempt, under a phase budget where re-asking spends what the snapshot behind it needs; this one runs on the happy path, where a re-ask costs a suite nothing and buys it the mirror. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
04494555c9 |
test(e2e): pin that an unanswered mirror lookup is not a mirror absence
The Talos image factory decision asks the apiserver once whether the in-cluster mirror Deployment exists, reads any non-zero exit as an absence, and caches that answer for the whole suite. A refused, throttled or timed-out lookup therefore sends every tenant worker in the run to the public Image Factory over live egress -- the dependency the mirror exists to remove -- and the run reads as ordinary environment noise rather than as a decision that was never actually made. Route that question through one helper and pin what the helper owes its caller: the three outcomes it must tell apart, the flags that make the three-way answer possible at all, a deadline on every attempt, the caching rule -- a real absence is resolved once and cached, an unanswered lookup falls back for its own caller alone -- and that a missing local timeout binary leaves the lookup asking the apiserver rather than answering unknown three times over. Two of those need saying carefully, because both concern what the caller is told when the lookup fails. A killed attempt gets its own case, driven by a wrapper that exits without running kubectl at all, so the lookup has to account for why it gave up rather than relay someone else's words: pinning the reason on a kubectl that failed loudly, as the sibling case does, pins it only where it was never in doubt. And the wall-clock bound is pinned as strictly greater than the request deadline beside it, which is the property that lets kubectl reach its own deadline and say what happened; level with it, the kill always lands first. The helper is a placeholder in this commit and claims present unconditionally, so the cases that need the distinction fail. The stubs are files on PATH rather than shell functions because the lookup runs under an external timeout binary, which execs a real file and would never see a shell override. xtrace stays off for the rest of a test that captures stderr: turned back on, a failing assertion wedges bats instead of reporting it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
048088bd29 |
fix(e2e): bound the tenant failure diagnostics per read and as a phase (#3676)
<!-- 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 The node-join failure diagnostics in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` issued about a dozen `kubectl` reads with neither an outer wall-clock bound nor `--request-timeout`, starting with a `describe nodes` through the tenant kubeconfig. That read targets the component least likely to answer in a node-join failure. An unbounded read there does not lose only itself: it holds the Chainsaw op until the op is killed, so everything scheduled after it is lost rather than truncated, the tenant crust-gather snapshot above all. Every read now carries the per-call bound the newer collectors on this path already use. A read that fails or is cut off says so instead of passing in silence, and says only what was observed, because a read that did not finish is silence about the cluster rather than a finding about it. The CDI importer walk gains a cap, and its listing is read on its own so a listing that never answered cannot read as a namespace with no importer in it. Per-read bounds alone do not end the phase, and the collectors here can together spend more than what is left of the op by the time they run. So the phase is bounded as a phase: once its budget is spent, the collectors that have not started are declined out loud. That is not a worst case and is not written as one, but it stops the snapshot being queued behind work begun past the deadline, which is how the snapshot was being lost. The op ceiling is unchanged, since a larger ceiling multiplied by a sum that grows with every collector added is the same problem one collector later. Because what runs last is what gets declined, the block is reordered so the cheap reads come first. The CSR list and the signer log are the discriminator for the failure mode a follow-up fix has to target, and behind two minute-scale collectors they were exactly what a tight run would drop. Adjacent work on the same failure path, all of it in service of the same requirement: - the sibling scheduling-gate branch ends in the same exit and depends on the same snapshot, so its two reads are bounded too; - `talos_image_cache_diagnose` gets its reads bounded and its gate gets `--ignore-not-found`, because `kubectl get` exits 1 for a refused connection or Unauthorized as readily as for NotFound, so a call that never reached the apiserver was being announced as a mirror that was never deployed; - the four numeric knobs are validated rather than only documented, since a suffix or a leading zero on the budget reached the arithmetic in the block's first statement and unwound the whole function before its own headline, leaving a failing run with no diagnostics at all. Comments carry invariants rather than arithmetic. A comment that states a sum has to be kept true forever in every copy of it, and any edit to the code or the order falsifies one of them. The numbers live in the code as values and in the guard that pins them. Relates to #3656. ### Known gaps, deliberately not closed here Some collectors on this path carry no wall-clock bound at all. The image-cache reachability re-probe makes seven unbounded management-cluster calls, and the guest-Talos capture walks its VMI rows uncapped with applies and waits bounded only by `--request-timeout`, which bounds one HTTP request rather than a client retrying against a wedged apiserver. The phase budget stops them being started late; it does not stop one running long once admitted. They are pre-existing and tracked in #3666. Bounding the re-probe in particular would change when the same function gives up on the happy path, and so which image factory tenant workers use, which is not a diagnostic's decision to make. Named improvements left for later, none of them regressions introduced here: - the phase deadline is stamped at block entry rather than against what is left of the op, so a long bringup still narrows the room the derivation assumes; - `bringup` and `largest` are literals in the guard, so a collector heavier than today's will not move them, and the margin is small by design; - the integer-knob validator now exists in four places in this tree, and a shared `_lib` file sourced ahead of both libraries would collapse two of them; - the per-request bound equals the wall-clock bound, so both deadlines fire together and which of the two notes appears is a race; - the ordered list in the chainsaw comments omits the ungated wedge check, which runs first; - section letters no longer sort after the reorder, and the comment-to-code ratio in the touched block is high. ### Testing `hack/run-kubernetes-node-join_test.bats` is new: 22 cases driving the block with a `kubectl` stub that hangs, refuses and answers in part, and a `timeout` stub that fails any read not wrapped in the bounding idiom. It pins that no read escapes a bound, that a cut-off read is named rather than reported as an empty cluster, that the walk cap reports both counts, that a failed listing is not reported as no importers, that the phase declines the rest out loud, that a malformed knob falls back and says so, that the block returns zero so the caller keeps its `exit 1`, and the source order that keeps the discriminating reads ahead of the expensive collectors. Every one was checked by mutation against the unfixed shape. ### 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): bound every read in the tenant Kubernetes node-join failure diagnostics and give the collection phase its own wall-clock budget, so a wedged apiserver can no longer hold the Chainsaw op until it is killed and take the tenant crust-gather snapshot with it ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved Kubernetes node-join and scheduling failure diagnostics with bounded collection times, clearer timeout and failure reporting, and more reliable preservation of critical snapshots. - Added safer handling for unavailable diagnostic data, missing tools, failed commands, and undeployed image caches. - Added configurable and validated timeout controls for image-cache and failure diagnostics. - **Documentation** - Clarified diagnostic budgets, collection ordering, and behavior when available time is exhausted. - **Tests** - Added comprehensive coverage for diagnostic timeouts, ordering, validation, failure handling, and bounded reads. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
86fb92abf5 |
docs(tests): correct five stale claims in the e2e capture tooling (#3662)
<!-- 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 Five things in the e2e tooling said something the code no longer does, or did not say it at all. All five are prose; no executable line changes. `docs/agents/e2e-testing.md` section 8 described two of the three on-failure capture legs. The missing one is `hack/e2e-capture-dataplane.sh`, the only leg that reads forwarding state off the node rather than object state, and the only one whose two callers bound it differently, 600s from the BATS runner and 300s from the Chainsaw catch. That split is what decides whether something added to it fits, so a contributor who reads the document and not the script sizes a change against the wrong budget. Its notes contract is described with the carve-out the script declares itself: the two EndpointSlice reads in the LoadBalancer path report nothing either way, so a `<none>` backend line there is the one place a reader still cannot tell an endpointless Service from a read that never answered. The same section said the runner's captures serve "the BATS tests" in two places. They serve the e2e ones. The runner keys on an `e2e-` prefix, on the suite's own filename or on its parent directory, because all three legs read cluster state and a unit runner has `kubectl` on PATH with no cluster behind it. The legs are not gated alike either, so the sentence says which fired where: two key on `kubectl`, and the snapshot leg keys on `crust-gather` being installed, which on a unit runner it usually is not. Two numbers in the capture-gate rationale in `hack/cozytest.sh` no longer described the code. The snapshot leg's cost was given as 360s, the deadline passed to `timeout`; the leg is invoked as `timeout -k 30 360`, so the trap waits out 390s, which is what the comment forty lines below and the op-budget arithmetic in `.chainsaw.yaml` already said. And the data-plane leg was still described as having no bound on its first pod list, and billed at the full 600s on that basis, after that read was given a 28s ceiling. Two files enumerated the BATS suites and both stopped one short. The global catch in `hack/e2e-chainsaw/.chainsaw.yaml` said the runner it names as its analog "only covers the BATS install/openapi tests", and `hack/e2e-chainsaw/README.md` named the same two files. Both miss `hack/e2e-prepare-cluster.bats`, which `packages/core/testing/Makefile` drives through that runner, and which is the suite whose failures leave the sandbox in the state hardest to reconstruct afterwards. `docs/agents/e2e-testing.md` already listed all three, so the tree disagreed with itself depending on which file you opened. A comment in `hack/e2e-install-cozystack.bats` pointed at `hack/e2e-apps/run-kubernetes.sh`, which does not exist; that directory holds two monitoring suites and no runner. The readiness gate it means is the `rollout status` wait in `hack/e2e-chainsaw/_lib/talos-image-cache.sh`, along with both fallbacks to the public factory. relates to #3626 and #3642 ### 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 NONE ``` |
||
|
|
9278702018 |
docs(e2e): make the op-budget comment name what it guarantees
The 50m ceiling on the kubernetes-* script ops is justified by a comment enumerating what the failure path can spend, and the enumeration was both incomplete and reassuring in the wrong direction. It listed some collectors, said nothing about the node-join diagnostics or the image-cache diagnosis, carried a figure for the guest Talos capture that was half its real cost, and closed by asking the reader to keep the op above the sum, which no ceiling can do because the sum runs past what is left of the op on its own. It now names the phase budget as the thing that keeps the snapshot reachable and lists the collectors this ceiling covers, in the order the phase spends it. It states no sums and no per-collector figures. Why the budget is that number is derived once, beside the budget, and pointed at from here: a figure repeated across three files is a figure that drifts, and the drift is what the reader trusts. It also says what the ceiling does not promise. The budget bounds when a collector may start rather than when the phase ends; several collectors on this path carry no wall-clock bound at all; and the bringup's own waits already exceed this op on ceilings before a single diagnostic runs. So no worst case is claimed, because none can be stated while that holds. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
0d7beaf630 |
fix(e2e): bound the tenant failure diagnostics per read and as a phase
The node-join diagnostics issued about a dozen kubectl reads with neither an outer wall-clock bound nor --request-timeout, starting with a describe nodes through the tenant kubeconfig. That read targets the component least likely to answer in a node-join failure, and an unbounded read there does not lose only itself: it holds the Chainsaw op until the op is killed, so everything after it is lost rather than truncated, the tenant crust-gather snapshot the failing exit triggers above all. Each read now carries the per-call bound the newer collectors on this path already use, sized lower because this block issues a dozen back to back where those bound one. A read that fails or is cut off says so instead of passing in silence, and says only what was observed: a read that did not finish is silence about the cluster, not a finding about it. The importer walk gains a cap for the same reason its listing is now read on its own, since a listing that never answered must not read as a namespace with no importer in it. Per-read bounds alone do not end the phase, and the collectors here can together spend more than what is left of the op by the time they run. Much of that is not ceiling-bounded at all: on the guest-Talos path the applies, waits and deletes carry --request-timeout or --timeout, which bound one HTTP request rather than a client retrying against a wedged apiserver. So the phase is bounded as a phase. Once its budget is spent the collectors that have not started are declined out loud. That is not a worst case and is not written as one, but it does stop the snapshot being queued behind work begun past the deadline, which is how the snapshot was being lost. Raising the op ceiling was the alternative and it is not one: a larger ceiling times a sum that grows with every collector added is the same problem later. Because what runs last is what gets declined, the block is reordered so the cheap reads come first. The CSR list and the signer log are the discriminator for the failure mode a follow-up fix has to target; behind a serial-console walk and a guest-Talos capture that can spend the whole budget between them, they were exactly what a tight run would drop. The comments carry invariants rather than arithmetic. A comment that states a sum has to be kept true forever in every copy of it, and any edit to the code or the order falsifies one of them. The numbers live in the code as values and in the guard that pins them; the prose keeps why the budget comes from an inequality, why admission gates the start rather than the finish, why the cheap reads run first, and which residuals the guard's literals do not cover. The sibling scheduling-gate branch ends in the same exit and depends on the same snapshot, so its two reads are bounded too. So are the reads in the talos-image-cache diagnosis, whose gate also gains --ignore-not-found: exit 1 from kubectl get means a refused connection or Unauthorized as readily as NotFound, and reading it as absent announced the mirror as never deployed on the strength of a call that never reached the apiserver. Bounding those dumps needed a note of its own first, since timeout prints nothing when it fires and a cut-off dump would otherwise leave a bare section header that reads as a cache with no Pod, no Service and no EndpointSlice. The knobs those bounds come from are validated rather than only documented. Each is pasted somewhere that takes digits and nothing else, and each failed differently and quietly: a suffixed or leading-zero budget reached the arithmetic in the block's first statement and unwound the whole function before its own headline, so a failing run produced no diagnostics at all; a suffixed read bound became a millisecond request timeout and made every note blame the cluster for a value; zero disabled the bound outright; a suffixed importer cap made the comparison exit non-zero, so the cap stopped existing; and a non-numeric grace makes timeout exit before running the command at all. A rejected value falls back and says so, which is what the rest of this tree does with its own knobs. All four are re-checked where they are used, since a value set after sourcing never passed the assignment, and the read bound is checked again before the per-request string is composed from it so the two halves cannot disagree. Each default is declared once as a constant, and the re-checks read the bare variable so an unset knob is named rather than silently defaulted. When timeout itself is absent the reads run unbounded rather than being bounded with a binary that is not there: the latter turns every read into an exit 127 and every note into the cluster refusing, and collects nothing, where unbounded is what this path did before any of this. The phase says so once, and says what it costs rather than reassuring. The reachability re-probe in the image-cache helper stays unbounded and its comment names all seven of the calls it makes, the fire-and-forget delete after the probe Pod included: the same function decides the mirror on the happy path, so changing when it gives up is not a diagnostic's decision to make. Its bounds are spelled at each call site rather than precomputed, since a prefix built at source time cannot follow a knob a test lowers afterwards while the --request-timeout beside it does. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c6961541f3 |
fix(tests): report a data-plane capture cut short by its backstop
Both callers of the data-plane collector wrap it in a wall-clock backstop and threw the result away with `|| true`, while the previous-instance logs leg sitting beside each of them records the exit status and prints "INCOMPLETE (exit N); kept what landed in ...". When a backstop killed the collector, nothing said so, and a truncated dataplane/ directory read exactly like a complete one that found little. The collector itself now names every read of its own it could not finish, which left these two call sites as the only remaining way for that leg to die without a word. The Chainsaw one is the tighter of the pair -- 300s against 600s, because it shares an op envelope with the snapshot leg -- so it is both likelier to fire and more expensive to leave silent. In each place the status is read, never propagated: a diagnostic still cannot change a test's outcome. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
b11066e770 |
docs(e2e): name every suite that stayed on the BATS runner
Two files enumerated the BATS suites and both stopped one short. The global catch in .chainsaw.yaml describes itself as the Chainsaw analog of the EXIT-trap host snapshot in hack/cozytest.sh, and said that runner "only covers the BATS install/openapi tests". The README said cluster bootstrap and the OpenAPI checks remain BATS and named only two files. Both miss e2e-prepare-cluster.bats, which packages/core/testing/Makefile drives through the same runner, and which is the suite whose failures leave the sandbox in the state hardest to reconstruct afterwards. docs/agents/e2e-testing.md already listed all three, so the count was answerable from the tree; it just disagreed with itself depending on which file you opened. The .chainsaw.yaml paragraph is reflowed because the correction is longer than the line it replaces; no other wording changed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c1071dab9b |
feat(kubernetes): let a node group log the guest serial console (#3637)
## What this PR does Both tenant Kubernetes e2e suites keep missing the node-Ready budget with zero Nodes registered, and the time goes inside the guest, between the worker VMI reaching Running and Talos asking for its apid certificate. Nothing collected today reaches that window. The in-guest capture the harness already has drives `talosctl`, so it needs the apid the worker never got to, and the management side has neither a Node nor a certificate request to read. Four attempts at this failure have been designed from outside the guest; the most recent raised the budget from 12m to 18m and the same fingerprint came back at the new deadline. KubeVirt can stream a VM's guest serial console into a `guest-console-log` container beside virt-launcher, readable with `kubectl logs` and needing nothing from inside the guest — no talosctl, no client certificate, no helper Pod, no reachable apid. That is the one property this failure class demands, so this turns it on for the suite's worker node group and collects it into the diagnostic bundle when node-join fails, ahead of the `talosctl` capture that cannot describe the same runs. Reads start at the beginning of the stream rather than tailing it: a guest that repaints its console would push the boot output out of any `--tail` window, while `--limit-bytes` truncates from the far end instead. Every read is bounded, and the walk stops at a cap that records both counts, so the collector cannot become an unbounded term in a failure path that already runs several others under one deadline. A read that returns no console is not filed as a quiet guest. Three outcomes get three labels, because only one of them says anything about the guest: the read failed and returned nothing, the read succeeded and returned nothing, or it broke off part way. Each read's stderr is kept beside the capture rather than inside it — kubectl's own `container guest-console-log is not valid for pod` would otherwise fill the file, and a console that never started would read as one that had spoken. That case is the headline one here, so the Pod state and events that explain it are collected alongside, once for the whole selector. The switch is a new opt-in `logSerialConsole` on a node group, off by default. It renders nothing unless set to true, including when set to false, because the `KubevirtMachineTemplate` is content-hash named and a byte emitted for a group that did not ask for it renames the template and rolls every live worker VM on upgrade. I verified that by rendering the base tree and this branch: absent and explicit-false produce a byte-identical template name on both charts. Worker VMs come from two charts, `kubernetes` and `kubernetes-nodes`; both carry the field, and the golden-parity check between them gains a case that keeps their output identical. The passing path checks separately that the container actually attached, and runs that check last, after everything the suite exists to prove — a debugging aid should not preempt the assertions that cover the cluster. Without it the suite would first learn the setting was inert in the one run that needed the console, when it can no longer be recovered. Relates to #3513. ### The risk this carries, stated up front The platform turns serial console logging off cluster-wide in the KubeVirt CR. That setting is a mitigation added in February 2026 for kubevirt/kubevirt#15989, where the `guest-console-log` container kept virt-launcher from starting and left VMs in `PodInitializing`. A per-VM `logSerialConsole` takes precedence over it — `isSerialConsoleLogEnabled` reads the VMI field first and only then falls back to the cluster setting — which is what makes this work, and also makes this the first thing on the platform to go through that mitigation. The mitigation was written against KubeVirt v1.6.x; the platform now ships v1.8.4, and the upstream issue is still open with no reproduction reported either way on the newer version. So the first run of these suites is also the experiment, and it doubles as a revalidation of that February mitigation on the version actually shipping, which nobody has done. The two outcomes are distinguishable rather than ambiguous: kubevirt/kubevirt#15989 shows up as virt-launcher wedged in `PodInitializing` with the VMI never reaching Running, while the failure this instruments shows the VMI Running and zero certificate requests. A red run with the first signature is an answer, not noise — and the collector is built so that it says which one happened instead of reporting silence. The blast radius is small: the product field is absent by default and only the e2e node group sets it, so backing this out is one line. The revert criterion, stated before the run rather than after it. If worker virt-launcher Pods sit in `Init` on `guest-console-log` and no Node registers, kubevirt/kubevirt#15989 still reproduces on v1.8.4: drop the `logSerialConsole: true` line from the e2e node group, keep the chart field, and the upstream issue gains the reproduction on a current version that its thread does not have. If the Pods reach Running and the console logs carry boot output, the mitigation is a candidate for removal platform-wide, which is its own change. Either way the run answers something that was open before it. Dropping `virtualMachineOptions.disableSerialConsoleLog` outright was the alternative. It is a smaller diff and needs no values key, but it exposes every VM on the platform to a race nobody has re-tested, instead of one node group in one test. That is its own revalidation exercise and belongs in its own change, on evidence this one produces. ### Verification `make unit-tests` passes. The new bats suite runs green under both `bash` and a `dash` `/bin/sh`, which matters because `hack/cozytest.sh` is `#!/bin/sh` and an earlier revision of this branch carried a bash-only construct that failed silently there. Every test in it is killed by a specific mutation of the code it covers. Both charts' helm-unittest suites pass, the golden-parity script reports byte-identical output across all four cases including the new one, and `make generate` is idempotent at the repo root. Reviewed by Claude across several rounds and by an independent second reader; the review loop found and fixed real defects in this branch, including a green-path check that had become unable to report the one thing it existed for. ### Screenshots Not a UI change. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/26 - [ ] [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 feat(kubernetes): worker node groups accept `logSerialConsole`, which streams a worker VM's guest serial console into a `guest-console-log` container readable with `kubectl logs`. Off by default. It is the only way to observe a worker that stalls during boot before Talos apid answers, when no Node and no certificate request exist to diagnose from. Enabling it rolls the node group and overrides the platform's cluster-wide `disableSerialConsoleLog`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an optional `logSerialConsole` setting for Kubernetes worker node groups. * Streams worker VM guest serial-console output to a dedicated log container when enabled. * Configuration is disabled by default and available through Kubernetes application settings. * **Bug Fixes** * Improved worker failure diagnostics with bounded serial-console output and pod-state information. * Validates that enabled worker pools attach the expected console-log container. * **Documentation** * Documented configuration behavior, rollout effects, defaults, and timeout considerations. * **Tests** * Added coverage for rendering, attachment validation, capture limits, and failure scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
50c5e2c14c |
test(e2e): capture the tenant worker guest serial console
The tenant Kubernetes suites fail the node-Ready budget with zero Nodes registered, and the time is lost inside the guest between the worker VMI reaching Running and Talos requesting its apid certificate. Nothing collected today covers that window: the existing in-guest capture drives talosctl, so it needs the very apid the worker never reached, and on the management side there is neither a Node nor a certificate request to read. Turn on the guest console log for the suite's node group and collect each virt-launcher's guest-console-log container from the management cluster. Reads start at the beginning of the stream rather than tailing it, so a guest that repaints its console cannot push the boot output out of the window; every read is bounded, and the walk stops at a cap that records both counts, so the collector cannot become the unbounded term in a failure path that already runs several others under one deadline. A read that returns no console is not filed as a quiet guest. Three outcomes get three labels, because only one of them says anything about the guest: the read failed and returned nothing, the read succeeded and returned nothing, or it broke off part way. Each read's stderr is kept beside the capture rather than inside it, since kubectl's own "container is not valid for pod" would otherwise fill the file and a console that never started would read as one that had spoken. That case is the headline one here, so the Pod state and events that explain it are collected alongside, once for the whole selector. Enabling the console overrides a cluster-wide setting the platform holds for an open upstream defect, so the run is also an experiment, and its own failure mode is the one that hides best: the container wedges virt-launcher in Init, no worker boots, and the suite reports "fewer than 2 tenant nodes Ready" -- the same sentence the failure under study produces. Read as the known signature, the experiment's answer would be filed as the flake it was meant to explain. So the failure path names that case first, before any other diagnostic, and only when it is true. The passing path checks separately that the container attached at all, and reports the ratio it saw rather than a bare success, since the check is deliberately satisfied by any one Pod. It keeps three outcomes apart too: a read that did not answer, a selector that matched no Pod, and Pods that matched and carry nothing. It reads the Pod name alongside the container names so the last two cannot collapse, and both container lists so a future move of the container cannot fail a run over a diagnostic. It runs last, after everything the suite exists to prove, because a debugging aid must not preempt the assertions that cover the cluster. The capture runs before the talosctl one because it is the only of the two that can describe a worker which never reached apid, the shape this failure keeps taking. kubectl reports a warning and an empty result alike, on stderr with a zero status, so the exit status decides which artifact its output lands in. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
bc6d5e67b7 |
fix(e2e): keep failure snapshots complete and gate tenant patch on aggregated-API auth (#3444)
## What this PR does Two unrelated defects made the sandbox install job's failure handling unreliable. This fixes both. The first hit every red run. When a test fails, the diagnostic snapshot runs `crust-gather collect`, and its host-log leg spawns a privileged debug pod per node. The sandbox runs baseline PodSecurity, which rejects each of those pods with a 403, so crust-gather keeps retrying until it burns its whole collection budget and exits 1. The snapshot verdict then printed `FAILED` on every failed run even though the namespaced object state was already captured, so the line that says whether the uploaded snapshot is trustworthy was always wrong. The host-log leg collects nothing usable here anyway, so this passes `--disable-additional-logs` at all three collect sites: the host snapshot from the BATS runner, the host snapshot from the Chainsaw error handler, and the nested tenant snapshot. Object collection now finishes and the verdict again separates a real collection failure from a complete snapshot. The second is a race in the install test. The "Configure Tenant" step issues its first `kubectl patch` of the root tenant as soon as an earlier test sees the aggregated APIService go `Available`. That condition only tracks endpoint availability, not the serving pod's auth state. A freshly rolled cozystack-api pod can start answering before it has loaded the requestheader client-CA, and until it does it drops the front-proxy identity and answers as `system:anonymous`, so the patch is rejected with a 403 on the test's very first command. This adds a short authenticated GET against the same resource before the patch, so the step proceeds only once the aggregated API both serves and authenticates the request. If the aggregation layer is actually wedged past two minutes the gate fails loud instead of hiding a real regression. Both were traced from a recent sandbox failure (run 30004030331, during PR #3441's CI). Out of scope: the broader convergence flakiness of the install step is not touched here. That covers the long reconcile windows for the storage and platform releases and the app-level races in the Chainsaw suite. The durable product-side fix for the second defect, where cozystack-api should not report ready before the requestheader client-CA is loaded, is a separate follow-up. This partially addresses #3368, its crust-gather diagnostics item, and does not close it. ### 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. --> No UI changes. ### 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. --> The diff touches only the e2e test harness (`hack/cozytest.sh`, `hack/e2e-chainsaw/.chainsaw.yaml`, `hack/e2e-chainsaw/_lib/run-kubernetes.sh`, `hack/e2e-install-cozystack.bats`). The trigger map's `hack/` couplings are `hack/e2e-prepare-cluster.bats` (node prerequisites for ansible-cozystack and talm) and `hack/package.mk`, `hack/common-envs.mk`, `hack/update-crd.sh` plus the package layout (ccp, external-apps-example, cozyhr). None of those are moved, renamed, or edited here, and nothing else in the map matches. - [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): keep failure snapshots complete when PodSecurity blocks crust-gather's host-log leg, and wait for the aggregated API to authenticate before patching the root tenant so the install step no longer flakes on a system:anonymous 403 ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved failure diagnostics during automated Kubernetes and tenant tests by avoiding unsupported privileged log collection steps. - Ensured failure snapshots complete more reliably instead of timing out or returning incomplete results. - Added a readiness check before configuring the root tenant, preventing intermittent authorization failures during installation tests. - **Tests** - Increased reliability of installation and end-to-end test workflows under baseline security policies. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
318be935ca |
fix(e2e): capture Talos logs for tenant worker join failures (#3548)
## What this PR does Addresses #3513. When tenant Kubernetes workers miss the 18-minute Ready deadline, the E2E runner now mints a one-hour `os:reader` Talos client certificate from the tenant's existing issuer and runs authenticated `talosctl` from a hardened same-namespace diagnostics Pod. It captures bounded `dmesg` and kubelet output for every worker VMI in the cozyreport snapshot, records missing IPs and command exit codes, continues across unreachable workers, and preserves the original test failure. The diagnostics Pod has no service-account token or Secret volume; credentials are streamed into an `emptyDir`, and the Certificate, Secret, and Pod are labeled for cleanup. Workers that have not reached Talos `apid` can only yield a bounded connection error, while workers that reached `apid` provide guest logs. This adds the evidence needed to distinguish the remaining guest boot failure but does not claim to resolve its underlying cause. The readiness budget and success path are unchanged. Because this changes a shared E2E helper, test-impact analysis selects the full E2E suite for this PR. ### Screenshots Not applicable; no UI changes. ### Testing - `hack/cozytest.sh hack/run-kubernetes-talos-diagnostics_test.bats` (10 tests) - `hack/cozytest.sh hack/run-kubernetes-drain_test.bats` - `hack/cozytest.sh hack/cozyreport-talos.bats` - `bash -n hack/e2e-chainsaw/_lib/run-kubernetes.sh` - `git diff --check origin/main...HEAD` The full `make bats-unit-tests` run reached the unrelated `hack/migration-seaweedfs-db-adopt.bats` suite and could not continue because Docker is unavailable in the development environment. A live failure-path run was not performed because the available development environment has no Talos tenant fixture; this PR is intended to exercise that path in Cozystack CI. ### 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): capture authenticated Talos kernel and kubelet diagnostics when tenant Kubernetes workers miss the Ready deadline ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved troubleshooting when worker nodes fail to join by collecting per-node operating system and kubelet diagnostics. * Diagnostics now continue across partial failures and clean up temporary resources afterward. * Extended Kubernetes test operation timeouts to allow failure diagnostics to complete reliably. * **Tests** * Added regression coverage for diagnostic collection, timeout handling, multi-worker scenarios, missing node addresses, and cleanup. * **Chores** * Ensured the required diagnostic image is preloaded on nodes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
0dae3ca95a |
fix(e2e): budget tenant scheduling separately from workload readiness (#3579)
## What this PR does The tenant backend Deployment in the kubernetes suites had one 300s budget to reach `condition=Available`, and two unrelated variable costs shared it. The first is scheduling. What the suite establishes before that point is two tenant nodes Ready, which is weaker than schedulable: a Ready node still carries `node.cilium.io/agent-not-ready` until the tenant cilium agent claims it, and a node the bringup has not finished with is `SchedulingDisabled`. In the run recorded in #3577 scheduling took 2m18s and 1m57s in the two suites, which left the image pull to finish inside what remained. It did not, and both suites reported the same `timed out waiting for the condition` for what were two different shortfalls. So this waits for a node that actually accepts a Pod before creating the workload, on its own budget and its own failure message. The gate encodes the scheduler's rule for a Pod that tolerates nothing (Ready, not unschedulable, no `NoSchedule` or `NoExecute` taint) over a `custom-columns` probe, prints the node table on both outcomes so a timeout names the taint that held it, and treats a failed probe as not-schedulable so an API blip cannot release it. On the happy path it adds no wall time: it spends the seconds the Pod would otherwise spend Pending, plus at most one 5s poll interval. On a failing run the two budgets stack, so a run that exhausts both now gives up at ~600s where it used to give up at 300s, inside the 40m Chainsaw script op the suites document as a ~25m bringup. The readiness wait keeps its 300s, now starting from a schedulable node, and dumps deployment, pod and event state when it runs out. The workload image is also pinned by digest. The tenant workers reach no Docker Hub mirror (`hack/e2e-talos-image-cache.yaml` serves the Talos worker OS disk image over HTTP and is not a registry mirror), so nginx is pulled from Docker Hub on every run either way. The digest does not take the pull off the critical path; it fixes what that pull returns instead of leaving a floating tag free to change size and content under a fixed deadline. Preloading the image would need infrastructure this tree does not have, and is not attempted here. Nothing will bump the pin: no renovate manager reads `hack/`, and the comment says so, because for a throwaway test workload the freeze is the point. What this does not claim: it removes two measured consumers from a fixed budget, both taken from the events of a single run. Whether that budget was the only thing making those suites red is not established from one run, so this is not offered as the fix for a red pipeline. `hack/run-kubernetes-schedulable_test.bats` covers the new logic: every branch of the predicate, the multi-node scan, the poll-again path, the deadline, and a probe that fails. Each test was verified by mutating the helper and checking that the intended test, and no other, went red. Note for whoever reviews alongside #3575: that branch mirrors `ghcr.io` for tenant worker pulls, not `docker.io`, so it does not change the Docker Hub pull described here. `git merge-tree` reports no conflict between the two, nor with #3548. Observed in #3577. ### Screenshots Not a UI change. ### 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: The trigger map was walked against the diff. The change is confined to `hack/e2e-chainsaw/_lib/run-kubernetes.sh` and one new `hack/*.bats` unit test. It moves and renames nothing under `hack/`, changes no make target, and does not touch `hack/e2e-prepare-cluster.bats`, any package, any CRD or any namespace name, so none of the listed repositories are reached. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Kubernetes deployment readiness by waiting for a suitable, schedulable node before timing backend startup. * Added clearer diagnostics when nodes are unavailable or backend readiness fails. * Replaced the floating backend container image tag with a fixed, verified version. * **Tests** * Added coverage for node readiness, cordoning, taints, polling, timeouts, probe failures, and deployment sequencing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
2527e915bd |
fix(e2e): collect diagnostics for unready pods and record every truncated capture (#3567)
## What this PR does The e2e diagnostic tarball exists so a red run can be explained from the uploaded artifact alone. Several of its collectors answered "nothing is wrong" instead — some because they selected on a column that never carries the value they filtered for, others because they stopped early and left nothing saying they had. Both failure modes look identical to a healthy cluster from the outside, which is the worst possible shape for a diagnostic. Four selectors in `hack/cozyreport.sh` and `hack/cozyreport-summary.sh` were reading the wrong thing: - Per-pod evidence was chosen by the STATUS column, so a pod left at `0/1 Running` by a readiness probe that never passes was skipped. That is a common way for a suite to time out, and the artifact carried no trace of the pod the run died on. Selection now compares the two halves of the READY column; terminal `Succeeded`/`Completed` pods stay excluded. - The pending-LoadBalancer section filtered `$4`, which is CLUSTER-IP. CLUSTER-IP holds an address, `None` or `<none>` and never `<pending>`, so that loop had never executed. `<pending>` only ever appears in EXTERNAL-IP, `$5`. - The cozystack-apps section gated all three of its kinds on `kubectl get crd <kind>.apps.cozystack.io`. ApplicationDefinition is a CRD but in a different group, and Tenant comes from an aggregated APIService with no CRD to ask about, so the probe failed and the section never ran. - The kamaji section was gated on `cozy-linstor`'s `linstor-controller` deployment rather than kamaji's own, so a cluster without LINSTOR got no kamaji logs and no TenantControlPlanes — on exactly the runs that need them. The summary had three of its own: the HelmRelease listing printed `$5` where STATUS is the Ready condition's whole message, reducing every failed release to one word like "Helm"; the Flux sources listing used a jsonpath with a nested filter that kubectl rejects outright, so it printed nothing on every report ever taken; and pod AGE was read as `$6`, which shifts whenever RESTARTS prints the `7 (4m12s ago)` form — that is, for exactly the rows worth reading. The second half is bounding. The pod, VM, VMI and services walks now run under a count cap and a wall-clock budget, and every read is individually time-boxed. The point is not the bounds themselves but that each one names itself when it fires: a walk stopped by the cap or the clock leaves `COLLECTION-TRUNCATED.txt`, a pod list that never returned leaves `COLLECTION-FAILED.txt` instead of the empty tree a healthy cluster produces, a log cut short says so on its own last line, and a `--tail` bound is recorded beside the logs rather than inside them, since the platform's controllers log JSON per line and a trailing prose line costs a parser the whole file. A knob value the collector could not use is reported too, so a truncation note quoting `600` is never mistaken for the `10m` somebody actually set. Where `timeout` is absent the reads run unbounded rather than not at all, and the section says which of the two happened, because a missing local binary is not a statement about the cluster. One behaviour change worth stating plainly, because it is a trade and not a pure gain: the per-pod section used to be uncapped and read container logs whole, and it is now bounded by `COZY_REPORT_PODS_MAX` (40), `COZY_REPORT_PODS_BUDGET` (600s) and `COZY_REPORT_POD_TAIL` (2000). Below those thresholds the new tree is a strict superset of the old one — the old selector missed `Running 0/1` entirely and skipped logs for `Pending` pods. Above them the artifact is smaller than before, every drop is named in the tree, and `COZY_REPORT_POD_TAIL=-1` with a larger `COZY_REPORT_PODS_MAX` restores the old reach. Because widening the selection and capping the walk in one change could otherwise let a newly-in-scope pod evict one the uncapped walk always collected, the ordering splits by evidence class before namespace: pods that are not `Running` — the class the collector took before this — all rank ahead of pods that are `Running` but unready, so the cap can only ever drop a pod the old collector did not keep either. `hack/e2e-capture-previous-logs.sh` gets the same treatment: its notes now go to `capture-notes.txt` in the artifact as well as to the job log, and a pod list that never returned is reported as such rather than as "nothing restarted". That script now creates its own output directory before its first line of output, which makes the comment in `hack/e2e-chainsaw/.chainsaw.yaml` wrong about why the once-per-test guard there holds — the only change to that file is the corrected comment, saying the `-d` check keys on whether the catch block already ran rather than on what the helper found. The rest is test coverage — a new `hack/cozyreport.bats`, an extended `hack/capture-previous-logs.bats` — and the removal of `EXIT`-trap cleanup from the bats files those tests now cover. That trap is worth its own note: inside an `@test` body it replaces the one the `bats` binary installs for its own bookkeeping, so a test that then fails prints no TAP line at all. It does not appear as `not ok`; it disappears, and only the `# bats warning: Executed N instead of expected M tests` line at the end distinguishes the run from a green one. A guard test enforces the ban over a named list of files that grows as the remaining ones are converted. ### Testing Every collector this touches is now covered by unit tests that run without a cluster: a new `hack/cozyreport.bats` (166 tests) and an extended `hack/capture-previous-logs.bats` (34), driven through `hack/cozytest.sh`. Ten `hack/*.bats` files run green locally, 265 tests in total, with each file's `@test` count reconciled against its passing count rather than trusted to the exit status — that reconciliation is the point of the trap change, since an exit code answers "did anything fail" and never "did anything run". The guard test that bans `EXIT`-trap cleanup was checked by committing the violation rather than by reading it: a folded `trap 'rm -rf "$tmp"' \` + `EXIT` added to a file on its list turns the guard red and names that file, and removing it turns the guard green again. `shellcheck --shell=sh` over the three changed scripts reports strictly fewer findings than before the change (`hack/cozyreport.sh` SC2086 170 to 116, SC2162 15 to 13; the three SC2102 in `hack/cozyreport-summary.sh` are gone), and `hack/e2e-capture-previous-logs.sh` is clean. The parts that need a cluster — that the bounds fire, and that each one lands in the artifact — are exercised by the e2e run itself, since these collectors run on every failed suite. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. This PR touches `hack/cozyreport.sh`, `hack/cozyreport-summary.sh`, `hack/e2e-capture-previous-logs.sh`, `hack/e2e-chainsaw/.chainsaw.yaml`, ten `hack/*.bats` files and `docs/agents/e2e-testing.md`. Nothing is moved or renamed under `hack/`, no make target changes what it does, and the files downstream consumers key on (`hack/package.mk`, `hack/common-envs.mk`) are untouched. No package, values key, CRD, variant, namespace or release asset is involved. - [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 diagnostic report now collects per-pod evidence for pods that are Running but not Ready, which it previously skipped, and several collector sections that silently produced nothing now report correctly. Collection that stops early on a bound now says so inside the artifact, so a truncated report can be told apart from a healthy cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Diagnostic reports now prioritize relevant resources and collect bounded pod, VM, service, storage, certificate, and Flux data. - Reports preserve partial results and clearly identify timeouts, unavailable resources, truncation, warnings, and collection failures. - Previous-container logs now include bounded capture, namespace prioritization, failure details, and partial-read evidence. - Reports target the operator workload and safely handle missing dependencies or temporary storage failures. - **Documentation** - Expanded guidance for reliable end-to-end testing and diagnostic report collection. - **Tests** - Added comprehensive coverage for report generation, log capture, failure handling, limits, and portability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
5542ae52b7 |
fix(e2e): budget tenant scheduling separately from workload readiness
The tenant backend Deployment had a single 300s budget to reach condition=Available, and two unrelated variable costs shared it. The first is scheduling. What the suite establishes before that point is two tenant nodes Ready, which is weaker than schedulable: a Ready node still carries node.cilium.io/agent-not-ready until the tenant cilium agent claims it, and a node the bringup has not finished with is SchedulingDisabled. Scheduling took 2m18s and 1m57s in the two tenant suites of one run, leaving the image pull to finish inside what was left. It did not, and both suites failed with the same message for two different shortfalls. Wait for a node that actually accepts a toleration-free Pod before creating the workload, on its own budget and its own failure message. The gate encodes the scheduler's rule for such a Pod (Ready, not unschedulable, no NoSchedule or NoExecute taint) over a custom-columns probe, prints the node table on both outcomes so a timeout names the taint that held it, and treats a failed probe as not-schedulable so an API blip cannot release it. The readiness wait keeps its 300s, now starting from a schedulable node, and dumps deployment, pod and event state when it runs out. Pin the workload image by digest. The tenant workers reach no registry mirror, so nginx is pulled from Docker Hub on every run either way; the digest fixes what that pull returns instead of leaving a floating tag free to change size and content under a fixed deadline. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
3fc8e524b2 |
fix(e2e): select catch pod logs by the nested release instance label
The harbor, openbao and qdrant app charts deploy their workload through a nested HelmRelease named `<release>-system`, and pass `fullnameOverride: <release>` down to it. The inner chart therefore stamps app.kubernetes.io/instance from its own release name, with the suffix, while object names come from the override, without it. The catch blocks selected on the outer release name, so podLogs matched no pods and every failed run collected an empty result. A pod name says nothing about its instance label; the two come from different sources on purpose. In qdrant the unsuffixed value is therefore not retired but kept as a collector of its own: the parent chart's PVC-cleanup Job labels its pod template with the parent release name, and that suite deletes the parent inside a `try:`, so the Job is in scope while the test still runs. The comment there is explicit about how narrow that window is -- the hook always exits 0, so Helm calls it succeeded and `hook-succeeded` takes the pod away; what is left is an uninstall that hangs with Helm still waiting on it. Harbor renders the same hook and does not get the same collector. It has no `delete` op, so its parent release is uninstalled only in Chainsaw's cleanup phase, which no catch can observe: catch is a defer inside the step, while the cleaner runs from t.Cleanup, after the test function returns. That collector would be the same permanently-empty capture this commit removes. A comment records the asymmetry so the next reader does not restore it for symmetry. openbao renders no such hook at all. Each workload value is now also pinned on a resource in an assert the suite already runs, so the chart renaming a label fails a step instead of quietly emptying a capture. The comments state what that does and does not cover: it does not catch a wrong value typed into a selector, and it catches nothing when the run fails before the assert. Harbor's database was reached by no collector at all. It is selected on `cnpg.io/cluster`, CNPG's own primary label, rather than on the instance label CNPG merely derives from the Cluster name -- the primary label has nothing to drift from, and it is what the postgres suite and the WorkloadMonitors already use. Each lookup keeps stderr apart from the value it captures. kubectl exits 0 while logging a klog line about an API group it could not reach, and an aggregated APIService in that state is what a failed run usually looks like; folded into the value, that line makes the "nothing found" branch unreachable and prints as though it were a result. Telling those outcomes apart is one helper in hack/e2e-chainsaw/_lib/ rather than a copy per call site, separated from the kubectl call so it can be pinned without a cluster, the way etcd-probe.sh already is. Both the census and the qdrant cleanup listing report through it, differing only in wording, so the discrimination is implemented and tested once. The tests cover each outcome for both wordings, including the zero exit with a klog line on stderr -- the case a folded stream turned into a row. A collector that matches nothing cannot say so: podLogs is silent, and `kubectl get -l` exits zero on an empty match. Each test-wide catch prints the instance label of every pod in the namespace, and the qdrant cleanup catch reports its three outcomes apart from a single call -- printing what that call returned rather than running it again, which could put a healthy listing under a failure heading. That census runs on a degraded cluster, where kubectl otherwise retries discovery against an unreachable apiserver until something kills the script, so every lookup carries `--request-timeout`. Measured against an unreachable apiserver, a bounded call costs about five times the flag, since kubectl retries discovery five times before it gives up. Each script then carries an explicit op timeout sized above its own calls -- 2m for the single-call censuses, 5m for the qdrant cleanup listing, which makes three. An inner budget is what should fire: a SIGKILL at the op boundary yields nothing, where a kubectl that gives up leaves an error to print. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
d3ae21b7f0 |
fix(e2e): name every way the previous-instance capture falls short
The capture wrote its reasons to the job log only. A reader holding the uploaded artifact saw N logs in a directory with no way to tell N from all of them: the container cap that dropped the rest, a pod list that never returned, a previous instance that produced nothing, and a read the outer backstop cut short all looked alike from outside. Every one of those lines now goes to capture-notes.txt as well, and the directory is created before the first of them, so a capture that found nothing is distinguishable from one that never ran. A pod list that failed is no longer reported as nothing having restarted. That is a claim about the cluster which the script never observed, and it is the claim most likely to talk a triager out of the crash-loop hypothesis on exactly the run where it is right. Exit 137 no longer asserts whose signal ended the read. 124 comes from timeout and from nothing else, but 137 is 128+SIGKILL, which the kill grace produces and so does the OOM killer or a teardown signalling the process group. Both belong in the cut-off branch, since the capture ends early either way, but reported flatly as the script's own timeout a read killed at second two reads as one that waited the full eighteen. COZY_PREVLOG_TAIL rejects a value kubectl cannot use and says so. A zero tail asks for no lines while kubectl still exits 0, which the empty-output branch would otherwise report as the container having logged nothing, discarding the only remaining copy of a log the kubelet has already collected. A leading zero is rejected because `$(( ))` reads it as octal while `[` reads it as decimal, so one string means two numbers. Each kept log names the bound it was written under, since --tail drops the oldest lines and nothing else in the output says so, and -1 suppresses that note because no bound was applied. An output directory that cannot be written is reported as the local condition it is. It arrives as an argument, so it can be a file, or missing, or unwritable; the redirection then fails before kubectl runs and hands the shell's own status to the branch that names kubectl, which reported "no previous instance retrieved (kubectl exit 2)" once per container. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
0612a538a3 |
fix(e2e): bound the Talos-capture failure-branch forensic kubectl calls
The describe/get-events/get-certificaterequests calls that run when the reader Certificate or the diagnostics Pod misses its readiness wait were unbounded, yet those branches fire precisely when the management apiserver is wedged. A stalled forensic call there can burn minutes of the 50m step budget before the pre-existing (a2)/(c) diagnostics and the EXIT-trap tenant snapshot run. Add --request-timeout=30s to them, matching the happy-path capture calls. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
cfbfd770b8 |
fix(e2e): bound the tenant Talos capture kubectl calls
The in-guest diagnostics chain runs only when the sandbox is already degraded enough to miss the 18m node-join deadline (the documented management-etcd fsync-storm regime), yet its kubectl legs were unbounded while every talosctl leg is wrapped in timeout. A stalled apiserver call in the (b) capture could consume the remaining step budget before (a2), (c) and the EXIT-trap tenant crust-gather run, so a diagnostics feature could reduce total diagnostic yield under the exact conditions it targets. Add --request-timeout=30s to the raw Certificate/Pod apply, the three reader-Secret reads and the worker VMI list, and route the Certificate Ready wait through kubectl_wait_retry so a single transient etcd-leader flap retries instead of aborting the whole capture. Update the bats assertion for the reworded wait call. Also correct a stale comment: the chainsaw step timeout is 50m, not 40m. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
60e44b24f1 |
fix(e2e): make tenant Talos diagnostics reliable
Fix admission, capture setup, timeout headroom, and portability. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
80152b1a07 |
fix(e2e): capture Talos logs for tenant worker join failures
Tenant Kubernetes worker join failures currently stop at host-side VMI and CSR state because the worker-only topology does not materialize a tenant talosconfig. Mint a short-lived os:reader client certificate and run talosctl from a hardened same-namespace Pod after the Ready deadline. Preserve bounded dmesg and kubelet captures for every worker without masking the original test failure. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
0c14d6cf43 |
test(e2e): sharpen round-trip diagnostics and make teardown settle
Recommendations from the third review pass (which returned LGTM; none of these blocked, all three are cheap and make the failure paths honest). 1. The round-trip's catch named nothing. An unnamed `describe HelmRelease` in tenant-root dumps every release on the root tenant of a live cluster and buries the two that matter, so name them (mariadb-2-backup-roundtrip names its release for the same reason). Raised podLogs tail 100 -> 200, since the sidecar logs every action and an S3 error sits well back from the tail by the time a step fails, and added the Keeper Pods: chkeeper.yaml labels them `app: <release>-keeper`, so the existing app.kubernetes.io/instance selectors missed them entirely — and a source ClickHouse that never reaches a ready replica is usually Keeper failing to form a quorum, which explains itself only there. 2. SSL_CERT_DIR is now `/etc/ssl/cozy-s3-ca:/etc/ssl/certs`. The single-directory form works — Go reads the default bundle FILE regardless, which is what the earlier measurement showed — but that left the public-CA half of the contract resting on a file this image happens to ship. Naming the system directory costs nothing and survives a base-image bump that moves the bundle; verified against public AWS S3 that both forms still verify (403 auth, no x509). The mount path stays the bare directory, and the unit test now pins both halves separately — it caught the over-broad edit that had briefly made mountPath a colon list. 3. cleanup.sh returned while the uninstalls it triggered were still running, so the next suite (etcd) could start with helm-controller still draining this one, and the pre-clean could hand step 04 an application whose predecessor was still going away. It now waits for both ClickHouse releases and the Bucket to actually disappear, and — since the script deliberately runs without `set -e` so one absent resource cannot abandon the rest — records those failures and exits non-zero, rather than reporting a teardown that did not settle as success. wait_deleted polls `get` instead of using `kubectl wait --for=delete`, which errors on an already-absent resource: the normal case here, and indistinguishable from a real failure. Sized the two cleanup step budgets to 12m for the same reason the run step is 60m: cleanup.sh's waits now sum to 540s, which the previous 8m did not clear. The Keeper-sized-from-the-server-resources bug the review found outside this diff is filed as #3487. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
82aa967674 |
test(e2e): correct the round-trip budget and its stale comments
Second review pass on this branch.
1. The op timeout equalled the inner sum instead of exceeding it: the comment
enumerated 3900s of inner waits and then set 65m, and 3900s IS 65m — so there
was zero headroom for the applies and SQL calls between those waits, and the
claim of "room" was false. Sized the inner budgets against what CI actually
takes instead (the bucket suite completes in ~37s and a backup-enabled
ClickHouse plus Keeper reaches Ready in ~98s, measured on run 30353894332):
bucket claim/access 300->180 and the three job waits 600->480, summing to
3300s, under a 60m op timeout. Shrinking the inner budgets rather than
raising the ceiling also keeps this suite closer to its postgres and mariadb
siblings on a 180-minute job cap.
2. The strategy Pod's securityContext comment still justified root with "Pod
runs apk add at startup", which the previous commit made conditional — the
default path now runs the platform's pre-baked image and installs nothing.
Scoped the note to the alpine fallback and said explicitly that the field set
matches cozy-default-altinity, runAsNonRoot deliberately unset in both.
3. clickhouse-backup.yaml still referenced clickhouse-backup-{1-contracts,
2-roundtrip}: the file moved unchanged, so the Test rename never reached its
own header and a reader following either name found nothing.
4. wait_sts_ready's timeout diagnostic could not print anything. It selected
clickhouse.altinity.com/chi=${name#chi-}, which yields
clickhouse-<app>-clickhouse-0-0 while the label carries the CHI name
(clickhouse-<app>), and 2>/dev/null then swallowed kubectl's "No resources
found". Ask for the StatefulSet's own first replica by name instead and let
both streams through; verified it now surfaces the Pod events that explain
why the replica never came up.
Also dropped the clickhouse bats case added in the previous commit: it asserted
what the generic examples/backups/<app> -> <app> mapping already guarantees, so
it did not lock the invariant its comment claimed. That invariant is now
recorded where it belongs — in the mapping's own comment in select-e2e.sh.
The dedicated ch-backup-client image this example now inherits is tracked in
#3420; noted there that the command: override is load-bearing in two files.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
|
c1fd7db51d |
test(e2e): fold the clickhouse backup Tests into the app's own suite
Review findings on the previous two commits. 1. TIA no longer selected the suite that runs the harness. select-e2e.sh maps examples/backups/<app>/** to suite <app>, so with the round-trip living in a separate clickhouse-backup/ dir an edit to the harness selected `clickhouse` — which did not execute those scripts — while the suite that does was left out. Fixed the way postgres/mariadb/etcd already do it: both Tests move into hack/e2e-chainsaw/clickhouse/ as clickhouse-1-backup-contracts and clickhouse-2-backup-roundtrip. That also deletes every special case the earlier commits added (src_to_suites, suite_to_source, and the bats case that guarded them — both scripts are now byte-identical to main), and the suite list in hack/e2e-chainsaw/README.md stays correct because no dir is added. The bats case is replaced by one asserting the invariant that actually matters: an examples/backups/clickhouse edit selects the suite that runs it. 2. The docs claimed a fail-fast the harness did not have, and the budget was sized as if it did. wait_for_field polled to the full timeout with no terminal-value escape, and the HelmRelease/StatefulSet waits were bare `kubectl wait` with no existence backstop and no Stalled fail-fast. Ported wait_for_field's fail_value parameter and wait_hr_ready from the postgres helpers, added wait_sts_ready, and routed all eight waits through them. Verified against a fake kubectl: a Failed BackupJob now bails in 3s instead of 600s, a Stalled HelmRelease immediately. The inner budgets sum to 3900s, which the old 30m op timeout did not cover — a hang would have been SIGKILLed with no diagnostics, the failure mode .chainsaw.yaml calls strictly worse — so the timeout is now 65m with the arithmetic written out. 3. The strategy Pod ran `apk add --no-cache curl jq` on every backup and restore. Harmless while nothing in CI ran it; a per-PR dependency on the Alpine CDN once it does, and broken outright air-gapped — which is exactly why the shipped strategy bakes curl+jq into its image. Step 01 now reads that image off cozy-default-altinity and drops the apk add, falling back to alpine only when the platform strategy is absent. Also bounded the two polling curls with --max-time; a hung connection previously stalled the loop until the outer timeout. 4. 91-scenario-user-backup.md still documented the old tenant-test default. 5. .chainsaw.yaml claimed all app tests run in tenant-test, which the postgres and mariadb round-trips already contradicted before this branch. The round-trip header now also states its scope plainly: it exercises the example's strategy, not the shipped cozy-default-altinity. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
dac34b7469 |
test(e2e): actually run the clickhouse backup round-trip in CI
The round-trip Test was skipping: it exited 0 behind CLICKHOUSE_E2E_S3_ROUNDTRIP, so "PASS (1.01s)" meant the gate fired, not that backup and restore worked. Only the contracts Test ran, which proves the chart still exposes the sidecar and its auth Secret — not that a byte ever reaches S3 and comes back. The gate existed because the sidecar could not verify the in-cluster SeaweedFS certificate. backup.endpointCA closes that, so adopt the shape mariadb and postgres already use to run their round-trips un-gated: - examples: resolve the S3 endpoint from S3_ENDPOINT when set (BucketInfo advertises the external ingress URL, an unroutable placeholder in CI), copy the seaweedfs CA (auto-discovered from its cert-manager Certificate when the fullname default is absent) into a per-release Secret, and reference it from backup.endpointCA on both the source and the to-copy target. NAMESPACE now defaults to tenant-root, matching the other backup demos. - suite: drop the gate, run in tenant-root — an isolated tenant's Cilium egress allowlist blocks its Pods from reaching tenant-root's seaweedfs — and pre-clean with cleanup.sh, because tenant-root is shared and a leftover Succeeded BackupJob would satisfy the harness's wait without a backup running. cleanup.sh also drops the copied CA Secret; the platform's own is never touched. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
541060c508 |
test(e2e): assert clickhouse-backup sidecar auth-secret linkage
Extend verify-backup-sidecar to assert the sidecar's API_USERNAME/API_PASSWORD env resolve valueFrom the backup-api-auth Secret's username/password keys, not just that the Secret and the :7171 port exist independently. A chart change that repointed or dropped those secretKeyRefs while keeping both would slip through and surface only as 401s at BackupJob time — the silent break this suite exists to catch. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
dbfd56f695 |
test(e2e): guard clickhouse-backup TIA mapping + tighten suite
- Add a select-e2e_test.bats case asserting a packages/apps/clickhouse diff selects BOTH the clickhouse and clickhouse-backup suites (mirrors the kubernetes-application multi-suite test). Fails without the src_to_suites override that makes the backup contracts suite run on ClickHouse chart PRs. - Correct the backup-api-auth Secret assertion's prose to match what it proves (present with both keys, not "populated"). - Capture the to-copy restore target's Pod logs in the round-trip catch so a failure isolated to step 07 keeps its diagnostics. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
d8e003f4cb |
test(e2e): add clickhouse altinity backup chainsaw suite
Port the intended ClickHouse Altinity backup/restore e2e coverage from cozystack#2600 (never-merged hack/e2e-apps/backup-clickhouse.bats) to the current Chainsaw approach, reworked on origin/main where the Altinity backup feature and examples/backups/clickhouse already landed. Two Tests, etcd-suite shaped: - clickhouse-backup-1-contracts (CI): a backup-enabled ClickHouse, asserting the chart<->Altinity-strategy contract against the running cluster — the clickhouse-backup sidecar on the ch-backup-api :7171 port and the populated <release>-backup-api-auth Secret the strategy Pod authenticates with. Needs no reachable object store. - clickhouse-backup-2-roundtrip: the full Bucket -> BackupJob -> Backup -> in-place + to-copy RestoreJob round-trip, driving examples/backups/clickhouse/run-all.sh. Gated out of CI (CLICKHOUSE_E2E_S3_ROUNDTRIP=1) because kind's self-signed SeaweedFS S3 endpoint cannot be TLS-validated by the in-Pod sidecar. Wire clickhouse-application -> "clickhouse clickhouse-backup" in hack/select-e2e.sh so ClickHouse chart edits exercise the backup suite too. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
|
330acfe6e2 |
fix(e2e): widen tenant node-join deadline to 18m
The kubernetes-latest and kubernetes-previous suites flaked on the worker node-join wait under host load. In run 30260770694 both failed at exactly 12m with zero tenant Nodes registered and the tenant cilium HR still mid-install: a transient drbd.linbit.com/lost-quorum taint delayed the worker DataVolume imports, so the KubeVirt VMIs were only created ~2m into this wait and the guests then had ~10m to import Talos, boot, register a kubelet and let cilium turn the nodes Ready. This single deadline absorbs the entire worker bring-up (the machinedeployment replicas=2 gate clears before the VMIs even exist), so under storage pressure 12m is too tight. A less-loaded fleet run passed both suites unchanged, confirming load-induced slowness rather than a stuck bring-up. Widen the deadline to 18m, which restores margin and still sits well inside the 40m chainsaw step timeout. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
|
b22b7b2431 |
test(e2e): make kuberture split-horizon probes survive Cilium ingress-IP race (#3430)
## What this PR does The split-horizon-routing-proof step ran two external-dns probes as bare Pods and polled them by fixed name. When the Cilium host-scope IPAM plus Gateway API restore race (cilium/cilium#46799) hands a probe the per-node reserved:ingress IP, the sandbox is rejected with "IP already in use" and the e2e leak-healer deletes the wedged pod, expecting a controller to reschedule it onto a free IP. A bare Pod has no controller, so that delete was terminal: the probe vanished and the step failed on a NotFound pod. It surfaces as an intermittent kuberture E2E failure with a `FailedCreatePodSandBox` event on a probe pod. This runs the probes as Jobs. The Job re-creates its pod on a fresh IP after the healer deletes it, so the proof survives the race. Each Job is asserted to reach Complete instead of polling a fixed pod name, and its logs are read by the job-name label. The failure path also gains a catch podLogs selector for the probe pods, so their stdout is captured when a probe fails for a different reason. ### Screenshots None. No UI changes. ### Downstream repositories The diff touches only the kuberture E2E suite under `hack/e2e-chainsaw/kuberture/`. No package, values schema, chart, CRD or platform manifest changed. Nothing in the downstream trigger map is reached. - [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 NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved end-to-end test reliability by hardening the split-horizon routing proof against probe workload churn. * Strengthened route isolation validation between public and internal probes. * Enhanced failure diagnostics by capturing logs from the completed probe Jobs. * **Testing** * Updated DNS EDNS probes to run as Kubernetes Jobs (instead of bare Pods) to support retries and rescheduling. * Added explicit assertions that both public and internal probe Jobs reach **Complete** before running hostname/route isolation checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
436a19df1b |
test(e2e): make kuberture split-horizon probes survive Cilium ingress-IP race
The split-horizon-routing-proof step ran two external-dns probes as bare Pods and polled them by fixed name. When the Cilium host-scope IPAM + Gateway API restore race (cilium/cilium#46799) hands a probe the per-node reserved:ingress IP, the sandbox is rejected with "IP already in use" and the e2e leak-healer deletes the wedged pod, expecting a controller to reschedule it. A bare Pod has no controller, so the delete was terminal: the probe vanished and the step failed polling a NotFound pod. Run the probes as Jobs so the pod is re-created on a fresh IP after the healer's delete, assert each Job reaches Complete instead of polling a fixed Pod name, and read the logs by job-name label. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
8a44b3e408 |
fix(e2e): skip crust-gather host-log collection rejected by PodSecurity
The diagnostic snapshot captured on a failed e2e run spawns a privileged debug pod per node for its host-log leg. Baseline PodSecurity rejects every such pod with a 403, and crust-gather's retry loop keeps trying until it burns the entire --duration budget, then exits 1 with a deadline error. The snapshot verdict therefore printed FAILED on every failure capture even though the namespaced object state had been collected, and the host-log leg itself gathered nothing usable here. Pass --disable-additional-logs at all three collect sites: the host snapshot from the BATS runner, the host snapshot from the Chainsaw error.catch, and the nested tenant snapshot. The object collection now finishes cleanly, the happy path reports complete, and FAILED once again means a genuine collection failure. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |